Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Terminus attempts to reuse expired session token and fails, instead of establishing new session #2561

Open
deltafactory opened this issue Mar 26, 2024 · 10 comments

Comments

@deltafactory
Copy link

deltafactory commented Mar 26, 2024

Platform

( ) MacOS
(X) Linux
( ) WSL2

Installed Via

( ) Homebrew
( ) Apt Package
(X) Manually

PHP Version

(X) 7.4x
( ) 8.0
( ) 8.1

Terminus Version

v3.3.5

Command Executed

terminus -vvv wp SITE.live -- [WP CLI command and parameters]

Expected behavior

Terminus should either re-use the existing valid session or establish a new session using the machine token.

Actual behavior

This is running as a daily cron job. Normally the command works as expected. Every few days the job end in an error message:

[error] Could not locate a site your user may access identified by XXXXX: Invalid or expired session header: X-Pantheon-Session

The next daily run executes as expected. From what I can tell, a session is established and stored in .terminus/cache/sessions. The session expires in a few days from issue. Subsequent runs during the valid period do not extend the expiration date so that eventually an attempt is made after expiration. Terminus makes no attempt to use other stored machine-tokens to revalidate before exiting with the error provided.

@kporras07
Copy link
Contributor

Hi!

Do you have more than 1 machine token stored in this context?

@deltafactory
Copy link
Author

No

@jms-pantheon
Copy link
Collaborator

jms-pantheon commented Mar 26, 2024

I just hit this today as well, I do have two users however. I would expect a more helpful error message or a retry of my past session as well.

$ terminus install:run wp-ocp-20240326.dev ocp -vvv
 [info] #### REQUEST ####
Headers: {"User-Agent":"Terminus/3.2.2 (php_version=8.1.27&script=/usr/local/Cellar/terminus/3.2.2/bin/terminus)","Accept":"application/json","X-Pantheon-Trace-Id":"9e946ecf-acb7-bef1-8048-b1171a886423","X-Pantheon-Terminus-Command":"{\"command\":\"install:run\",\"arguments\":{\"command\":\"install:run\",\"site_info\":\"wp-ocp-20240326.dev\",\"job_id\":\"ocp\"},\"options\":{\"help\":false,\"quiet\":false,\"verbose\":true,\"version\":false,\"ansi\":null,\"no-interaction\":false,\"define\":[],\"yes\":false},\"truncated\":false}","Authorization":"**HIDDEN**"}
URI: [https://api.pantheon.io:443/api/site-names/wp-ocp-20240326](https://api.pantheon.io/api/site-names/wp-ocp-20240326)
Method: GET
Body: null
 [error]  Could not locate a site your user may access identified by wp-ocp-20240326: Invalid or expired session header: X-Pantheon-Session

$ cat ./session
{"session":"**HIDDEN**","expires_at":1711056917,"user_id":"**HIDDEN**"}% 

$ terminus whoami
 [notice] You are not logged in.

$ terminus machine-token:list
 ---------------------- ---------------
  ID                     Device Name
 ---------------------- ---------------
  dcr_LzAFI6O7X40IA5jy   terminus-demo
 ---------------------- ---------------

$ terminus auth:login
 [notice] Tokens were saved for the following email addresses:
**HIDDEN**@gmail.com
jordan.schulz@getpantheon.com
You may log in via `terminus auth:login --email=<email>`, or you may visit the dashboard to generate a machine token:
https://dashboard.pantheon.io/machine-token/create/C02G411LMD6V-jordanschulz

$ terminus auth:login --email=jordan.schulz@getpantheon.com
 [notice] Logged in via machine token.

@kporras07
Copy link
Contributor

Hi!

I think a good solution for scripts is to make sure you're logged in at the beginning of the script by running:

terminus auth:login --email=<your_email_here>

This should make sure you have a valid session.

We will create an internal ticket to investigate this behavior and see if there is a better solution for this

@kporras07
Copy link
Contributor

Every command tagged as @authorize in Terminus should make sure you're logged in and attempt to login if you have a single machine token:

public function ensureLogin()
    {
        if (!$this->session()->isActive()) {
            $tokens_obj = $this->session()->getTokens();
            if (count($tokens = $tokens_obj->all()) == 1) {
                $token = array_shift($tokens);
            } elseif (!empty($email = $this->getConfig()->get('user'))) {
                $token = $tokens_obj->get($email);
            } else {
                throw new TerminusException(
                    'You are not logged in. Run `auth:login` to authenticate or `help auth:login` for more info.'
                );
            }
            $token->logIn();
        }
    }

@deltafactory could you share the command that is failing for you?

@deltafactory
Copy link
Author

deltafactory commented Mar 26, 2024

I was wondering if excluding expired tokens during the getTokens() call shown above would help by failing the first check in the if statement and fall back to the $tokens_obj->get($email)

The call in question is a custom wp-cli command. I see that my syntax in the original issue text caused much of that to be eaten so let me fix it.

@deltafactory
Copy link
Author

@kporras07 Command in Issue desc fixed. The exact command is a proprietary command for a client. If we need a test case that others can replicate, we'll need to figure out something else.

@kporras07
Copy link
Contributor

Tokens in this context is machine token; we don't really have a way to know whether they're revoked or not (they don't expire but you could delete them from your dashboard).

If it is only 1 of them (or you have set a user email in your terminus config), it should use this token to reauthenticate. I just looked at the wp command and it has the correct @authorize annotation.

Could you please post the output of terminus machine-token:list ? (Feel free to mask the info you don't feel comfortable pasting here, please)

@deltafactory
Copy link
Author

The token I'm having issue with is the session token, not the machine token. I apologize if I'm not using the correct terminology. From the code excerpt, I thought $this->session()->getTokens() was saying this as well.

@kporras07
Copy link
Contributor

yeah, if a session is expired, for most commands that need authentication, it should use a machine token to generate it.

Could you please post the output of terminus machine-token:list ? (Feel free to mask the info you don't feel comfortable pasting here, please)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants