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

Refresh FusionAuth token #175

Closed
nfebe opened this issue May 15, 2023 · 12 comments · Fixed by #192
Closed

Refresh FusionAuth token #175

nfebe opened this issue May 15, 2023 · 12 comments · Fixed by #192
Assignees

Comments

@nfebe
Copy link
Contributor

nfebe commented May 15, 2023

When the file uploads would exceed an hour the backend would fail with Firebase\JWT\ExpiredException on GET /folder/getWithChildren

Leading to

2023/05/15 16:42:37 ERROR : nzmgdjdhzj: Failed to copy: Put mkParentDir failed: mkdir "/archives/rclone QA 1 (dev) (07av-0000)/My Files" failed: sftp: "" (SSH_FX_FAILURE)
2023/05/15 16:42:37 ERROR : nzndqkkgwb: Failed to copy: Put mkParentDir failed: mkdir "/archives/rclone QA 1 (dev) (07av-0000)/My Files" failed: sftp: "" (SSH_FX_FAILURE)
2023/05/15 16:42:37 ERROR : nynthjaqhy: Failed to copy: Put mkParentDir failed: mkdir "/archives/rclone QA 1 (dev) (07av-0000)/My Files" failed: sftp: "" (SSH_FX_FAILURE)
2023/05/15 16:42:38 ERROR : obmqbiapma: Failed to copy: Put mkParentDir failed: mkdir "/archives/rclone QA 1 (dev) (07av-0000)/My Files" failed: sftp: "" (SSH_FX_FAILURE)

On the sftp client.

Apparently the FusionAuth token expires after one hour and so it needs to get refreshed.

Sentry : https://permanentorg.sentry.io/issues/3899931004/?environment=dev&project=3663562&query=is%3Aunresolved&referrer=issue-stream&stream_index=0

@kfogel
Copy link
Contributor

kfogel commented Jun 1, 2023

See also related Slack thread.

@kfogel
Copy link
Contributor

kfogel commented Jun 1, 2023

Just moving some thoughts/commentary from team meeting notes docs over to here:

One option would be to just make the default tokens last longer (but "ick", for all the usual reasons).

Another option would be for the SFTP Service to refresh the FusionAuth token. In that case, refresh tokens would need to be enabled. Would the refresh token then also need to last longer than 12 hours, to allow for really long uploads? (Or can a refresh token be used to get a newer refresh token? That would be kind of suprising if it could, but who knows.)

See https://learn.microsoft.com/en-us/linkedin/shared/authentication/programmatic-refresh-tokens - could make them last a long time!

See also FusionAuth docs: https://fusionauth.io/docs/v1/tech/apis/jwt

@nfebe nfebe self-assigned this Jun 8, 2023
nfebe added a commit that referenced this issue Jun 20, 2023
Access tokens have lifetimes that might end before the user wants to end
their authenticated session. Hence, we need to refresh the user access token
after an expiry detection.

Resolves: #175

Signed-off-by: fenn-cs <[email protected]>
@jasonaowen
Copy link
Contributor

+1 to what @kfogel wrote: the initial OAuth request should ask for a refresh token, and every subsequent API request should examine the expiration time on the access token and, if near to expiry, use the refresh token to get a new access token and a new refresh token.

I don't know what the time limits for refresh tokens are configured to be here; that'll be in the FusionAuth configuration, and someone with access can confirm it for you: @cecilia-donnelly for sure, and maybe other staff developers too. In general, refresh tokens can be fairly long lived, on the order of weeks or months; this is safe because using them involves talking to the OAuth authority, which allows for re-validation and for token revocation. For example, if a user account were deleted, or they changed their password, or perhaps if the requesting IP address changed, the authorization server could reject the refresh request.

See also Okta's high-level documentation on refresh tokens.

@nfebe
Copy link
Contributor Author

nfebe commented Jul 6, 2023

Yes @kfogel! Thanks for linking that up.

Actually #194 was a futile attempt. My idea was that, by hitting the login endpoint directly, I would be able to ask for a refreshToken to come back in the current response.

In

this.fusionAuthClient.login({
loginId: this.authContext.username,
password,
}).then((clientResponse) => {

However, that did not seem to work and I later found in the documentation that we can't modify that behavior via the current login endpoint.

https://fusionauth.io/docs/v1/tech/apis/login#request

The issuer of the One Time Password will dictate if a JWT or a Refresh Token may be issued in the API response.

@kfogel
Copy link
Contributor

kfogel commented Jul 6, 2023

Got it, @fenn-cs; thanks. I removed my comment, by the way -- PR #192 had already been mentioned here via automatic cross-linking (because that PR mentions this issue), and, as you just explained, you're going to close PR #194 anyway.

@slifty
Copy link
Contributor

slifty commented Jul 14, 2023

Based on the conversation in #192 it looks like our current login mechanism does not support refresh tokens. That means we have the following two paths to resolution:

  1. Change our authentication flow to one that does support refresh tokens (or does not require tokens at all).
  2. Extend the life of the token to something long enough so as to obviate the issue.

Extending the life of a token is not a true fix, as there would still be edge cases where files take longer than the token's length to resolve.

Changing the auth flow

  • Oauth: In terms of changing the login flow we are a bit limited by rclone and sftp -- sftp doesn't support oauth directly, but does support open ended "keyboard response" authentication (which allows for general prompts and custom logic). We could use prompts to have users navigate to a website, login, generate a token, and enter the token as a prompt response. That being said, rclone does not support interactive prompts. Rclone expects the password to be set once at the point of client configuration (this is why we require users to disable 2FA to use rclone). Is there some way that oauth could result in a single token that the user is told to use during configuration? Is it possible that having users configure to use ssh-agent instead of entering a password would provide an avenue to interactive prompts?

  • SSH Keys: SFTP does support key based authentication. I believe this would NOT involve fusionauth and would require the permanent backend to allow users to add ssh keys to their accounts (and then APIs that allow access to public keys for a given user).

  • Something else? There are many other flows so I wanted a placeholder to represent the world of possibilities

@slifty
Copy link
Contributor

slifty commented Jul 20, 2023

UPDATE: it looks like it MAY WELL support refresh tokens!

We still may want to explore other forms of authentication at some point, but for now we're going to see if we can get refresh tokens working with the current approach.

@jasonaowen
Copy link
Contributor

I was able to obtain a refresh token via the login API, and then use the standard OAuth token endpoint to exchange it for a new access token. To do so, I needed to both pass in the application ID and ensure the user logging in was registered in the application. Per the login API docs:

Because a refresh token is per user and per application, this value will only be returned when an applicationId was provided on the login request and the user is registered to the application.

At the moment, we are not automatically registering users in the sftp application. FusionAuth was telling us that, but we weren't doing anything with that information:

SuccessButUnregisteredInApp = 202,

Setting the application ID is done by adding a key-value pair to the login object; the type definition does not include the key, but does not prevent it, either:

diff --git a/src/classes/AuthenticationSession.ts b/src/classes/AuthenticationSession.ts
index 9d43f3b..8b11b50 100644
--- a/src/classes/AuthenticationSession.ts
+++ b/src/classes/AuthenticationSession.ts
@@ -46,6 +47,7 @@ export class AuthenticationSession {
   private processPasswordResponse([password]: string[]): void {
     this.fusionAuthClient.login({
+      applicationId: 'redacted',
       loginId: this.authContext.username,
       password,
     }).then((clientResponse) => {

Renewing the token was a standard OAuth exchange:

curl -v \
  --request POST \
  --url 'https://permanent-dev.fusionauth.io/oauth2/token' \
  --header 'content-type: application/x-www-form-urlencoded' \
  --data "client_id=$(<client_id)" \
  --data "client_secret=$(<client_secret)" \
  --data "refresh_token=$(<refresh_token)" \
  --data 'grant_type=refresh_token' \
  | jq .

My next step will be to clean this up into a pull request, perhaps reusing some of the work in #192.

@slifty
Copy link
Contributor

slifty commented Aug 3, 2023

At the moment, we are not automatically registering users in the sftp application.

Ah! This makes sense! Will be good to talk through how to best resolve that.

Nice sleuthing! 🎉

@nfebe
Copy link
Contributor Author

nfebe commented Aug 3, 2023

Thanks for sleuthing (as Dan named it), @jasonaowen ! We discussed the following possible next steps during rclone check-in today.

Approach 1: Handle user-app association in sftp-service (i.e asking FusionAuth to do associations when we need them)

We are exploring the possibility of enabling the sftp service to automatically register any user who attempts to log in but is not already registered with the application in FusionAuth. This would involve handling the status 202, which currently indicates that the user is successful but unregistered in the app.

We this it is theoretically possible but would like to is to evaluate (check with you Jason) the security implications of this approach and whether it aligns with best practices. This way, we avoid all the multiple steps in approach 2 and the eminent should-not-happen case.

Approach 2: Handle user-app association in permanent-backend

  • Add an endpoint in the back-end to retrieve all current users associated with the sftp app in FusionAuth.

  • Update the registration process in the back-end to ensure that newly registered users are associated with the sftp app in FusionAuth.

  • Handle edge cases, where users are not associated with the app and thus do not receive a refresh token. For existing users who are not currently associated with the app. (We may consider running a script to make that association.)

  • Better error handling for edge cases if no script is run to fix all required but non-existent associations, for other cases where a refresh token is not provided due to a lack of app association, the user's session will still time out. However, our error handling will provide a clear and informative message to help users understand the cause of the timeout.

cc; @kfogel @slifty

@nfebe
Copy link
Contributor Author

nfebe commented Aug 5, 2023

More information on approach 1:

This API is used to create a new User Registration for an existing User. If the User has already created their global account but is now creating an account for an Application, this is the API you will use to create the new account. You must specify the User Id being registered on the URI. The Id of the Application the User is registering for is sent in the request body.

https://fusionauth.io/docs/v1/tech/apis/registrations#create-a-user-registration-for-an-existing-user

nfebe added a commit that referenced this issue Aug 5, 2023
Access tokens have lifetimes that might end before the user wants to end
their authenticated session. Hence, we need to refresh the user access token
after an expiry detection.

Resolves: #175

Signed-off-by: fenn-cs <[email protected]>
nfebe added a commit that referenced this issue Aug 6, 2023
Access tokens have lifetimes that might end before the user wants to end
their authenticated session. Hence, we need to refresh the user access token
after an expiry detection.

Resolves: #175

Signed-off-by: fenn-cs <[email protected]>
nfebe added a commit that referenced this issue Aug 6, 2023
Access tokens have lifetimes that might end before the user wants to end
their authenticated session. Hence, we need to refresh the user access token
after an expiry detection.

Resolves: #175

Signed-off-by: fenn-cs <[email protected]>
nfebe added a commit that referenced this issue Aug 6, 2023
Access tokens have lifetimes that might end before the user wants to end
their authenticated session. Hence, we need to refresh the user access token
after an expiry detection.

Resolves: #175

Signed-off-by: fenn-cs <[email protected]>
nfebe added a commit that referenced this issue Aug 20, 2023
Access tokens have lifetimes that might end before the user wants to end
their authenticated session. Hence, we need to refresh the user access token
after an expiry detection.

Resolves: #175

Signed-off-by: fenn-cs <[email protected]>
nfebe added a commit that referenced this issue Aug 30, 2023
Access tokens have lifetimes that might end before the user wants to end
their authenticated session. Hence, we need to refresh the user access token
after an expiry detection.

Resolves: #175

Signed-off-by: fenn-cs <[email protected]>
nfebe added a commit that referenced this issue Sep 8, 2023
Access tokens have lifetimes that might end before the user wants to end
their authenticated session. Hence, we need to refresh the user access token
after an expiry detection.

Resolves: #175

Signed-off-by: fenn-cs <[email protected]>
@kfogel
Copy link
Contributor

kfogel commented Sep 29, 2023

W00t! 🎉

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

Successfully merging a pull request may close this issue.

4 participants