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

feat: support SSO (Single Sign-On) #1010

Merged
merged 115 commits into from Nov 15, 2023
Merged

feat: support SSO (Single Sign-On) #1010

merged 115 commits into from Nov 15, 2023

Conversation

Integralist
Copy link
Collaborator

@Integralist Integralist commented Sep 12, 2023

This PR makes the following fundamental changes...

  1. Adds a new fastly sso command which uses OAuth (with PKCE flow) to authenticate a user with the Fastly platform. The returned "access" token (which is short-lived) will be exchanged for a short-lived API token. The access token will be automatically rotated using a "refresh" token until the refresh token itself has expired and then the user will need to re-authenticate to acquire a new access/refresh token pair.
  2. Updates the fastly profile create and fastly profile update commands to support the SSO/OAuth flow, and allows a user to switch between that token format and the traditional long-lived tokens that have only been supported in the CLI up until this point.

REVIEW NOTES

There are lots of changes in this PR and refactoring done to make maintaining the code easier, so I would suggest reviewing the following files (rather than looking at the diffs which will likely be very confusing).

  • pkg/app/run.go (specifically the processToken function)
  • pkg/auth/auth.go (handles the OAuth flow)
  • pkg/commands/profile/create.go
  • pkg/commands/profile/update.go
  • pkg/commands/sso/root.go
  • pkg/commands/sso/sso_test.go
  • pkg/config/config.go
  • pkg/env/env.go
  • pkg/global/global.go
  • pkg/testutil/args.go

ADDITIONAL NOTES

The CLI configuration file has its config_version field bumped to version 5 to coincide with some additional fields added to the config (e.g. [fastly.account_endpoint] and the token fields within [profile]).

Up until this point, there were a few CLI commands (not many) that would check (at the start of their invocation) for an API token and would return an error if no token was found. I've removed these checks and now, as part of the app.Run() function (which executes just before the actual command is invoked), we do a general token check/validation step (because with OAuth tokens we have additional processing that needs to take place).

TODO

We need to confirm whether the changes made to the fastly profile create and fastly profile update commands are considered 'breaking' or not. To explain, there is an additional interactive prompt added to both commands and so a customer outreach is required to validate whether these commands are being automated in any way within CI environments. I don't imagine they will be, and if they're not, then we shouldn't consider the extra prompt a breaking change.

SCREENSHOTS

The following screenshot shows my current profile configuration. i.e. I have one profile with a traditional 'long-lived' token...

1  one profile with legacy token

The following screenshot demonstrates what I would see if I invoke a command that requires an API token. Notice that the CLI informs me of my token being a non-SSO-based token and it also explains what I should do to switch to an SSO-based token.

BUT most importantly: the introduction of support for SSO doesn't break the experience for existing users. They can keep working (i.e. executing CLI commands), albeit with an extra message displayed informing them they should probably update their token.

2  whoami to show info message

⚠️ If the user uses an override like $FASTLY_API_TOKEN or --token or --profile then we skip checking for SSO token because the specified token might well not be configured within fastly config so we cannot determine TTLs, SSO status and things of that nature ⚠️


The following screenshot demonstrates that I can hide that message if I want using the --quiet flag...

3  quiet flag to hide info message

The following screenshot demonstrates what I would see if I decided to re-run my original command but also set the FASTLY_USE_SSO environment variable as originally suggested by the Fastly CLI. Notice the messaging is informing me that I've not authenticated before using OAuth/SSO and it's now prompting me to confirm I'd like to go ahead with authenticating...

NOTE: Ignore the extra --account and --endpoint flags, those are only present because I'm having to test against staging endpoints and so I have to set those flags to override the production endpoints that are the default.

4  set sso env var

The following screenshot demonstrates what a new user will see. A new CLI user will have no profiles configured (unless they have already run fastly profile create) and so in this scenario the CLI recognises there are no profiles (and no token overrides using the --token flag or the FASTLY_API_TOKEN environment variable), so it automatically informs me of this and will prompt me to authenticate using SSO.

5  new user

In all the preceding screenshots, I've let the CLI automatically handle the authentication flow (e.g. it determines if the existing SSO-based token has expired etc).

In the following screenshot I'm demonstrating what happens if I decide to execute the fastly sso command which will cause the CLI to authenticate directly.

The CLI notifies me of the profile it will be modifying, and once I authenticate, the token for that profile will be overwritten with a freshly minted token...

6  sso directly

The following screenshot demonstrates what happens when the 'access token' expires (its TTL is 5mins). What's interesting here is that I'm noticing every time we get a new access token, the 'refresh token' is updated as well (so even though the TTL for the refresh token is 30mins, the TTL is effectively refreshed every 5mins because every time we get a new access token we get a new refresh token)...

7  tokens expired

UPDATE: The INFO output has been moved behind the --verbose flag.

pkg/auth/auth.go Outdated Show resolved Hide resolved
@sgorbaty
Copy link

Refresh token expires in 30 min and keeps on getting refreshed up to 10 hours of max session life.

@Integralist Integralist marked this pull request as ready for review September 14, 2023 17:12
@sgorbaty
Copy link

Could you point me to or add the part that consumes the new refresh_token from /token endpoint and replaces the current? Thanks.

@Integralist
Copy link
Collaborator Author

@sgorbaty

Could you point me to or add the part that consumes the new refresh_token from /token endpoint and replaces the current? Thanks.

Sure!

In the CLI, before the user's requested subcommand is invoked, we grab the access token from the user's profile.

If the access token is expired, we next check if the refresh token has expired...

cli/pkg/app/run.go

Lines 279 to 280 in dd3e8da

// Refresh Token has expired
if auth.TokenExpired(profileData.RefreshTokenTTL, profileData.RefreshTokenCreated) {

If the refresh token hasn't expired, then we use it to get a new access token...

cli/pkg/app/run.go

Lines 291 to 302 in dd3e8da

updated, err := auth.RefreshAccessToken(account, profileData.RefreshToken)
if err != nil {
return tokenSource, warningMessage, fmt.Errorf("failed to refresh access token: %w", err)
}
claims, err := auth.VerifyJWTSignature(account, updated.AccessToken)
if err != nil {
return tokenSource, warningMessage, fmt.Errorf("failed to verify refreshed JWT: %w", err)
}
email, ok := claims["email"]
if !ok {
return tokenSource, warningMessage, errors.New("failed to extract email from JWT claims")
}

NOTE: I've just noticed ☝🏻 we also have the JWT validation logic here (so that's two places it exists) and so I need the azp/aud validation added here as well. So I'm going to sort that out today (I'm also going to refactor the logic so it only exists in one place!).

Next, we exchange the access token for an API token...

cli/pkg/app/run.go

Lines 304 to 309 in dd3e8da

// Exchange the access token for a Fastly API token
apiEndpoint, _ := g.Endpoint()
at, err := auth.ExchangeAccessToken(updated.AccessToken, apiEndpoint, g.HTTPClient)
if err != nil {
return tokenSource, warningMessage, fmt.Errorf("failed to exchange access token for an API token: %w", err)
}

Finally, we check if the refresh token (that we got back when asking for a new access token) is different from the refresh token currently stored in the user's profile, and if it is, we update the refresh token in the user's profile to be the refresh token that was returned in the call to refresh the access token...

cli/pkg/app/run.go

Lines 311 to 333 in dd3e8da

// NOTE: The refresh token can sometimes be refreshed along with the access token.
// This happens all the time in my testing but according to what is
// spec'd this apparently is something that _might_ happen.
// So after we get the refreshed access token, we check to see if the
// refresh token that was returned by the API call has also changed when
// compared to the refresh token stored in the CLI config file.
current := profile.Get(profileName, g.Config.Profiles)
if current == nil {
return tokenSource, warningMessage, fmt.Errorf("failed to locate '%s' profile", profileName)
}
now := time.Now().Unix()
refreshToken := current.RefreshToken
refreshTokenCreated := current.RefreshTokenCreated
refreshTokenTTL := current.RefreshTokenTTL
if current.RefreshToken != updated.RefreshToken {
if !g.Flags.Quiet {
text.Info(out, "Your refresh token was also updated")
text.Break(out)
}
refreshToken = updated.RefreshToken
refreshTokenCreated = now
refreshTokenTTL = updated.RefreshExpiresIn
}

@Integralist Integralist force-pushed the integralist/sso branch 3 times, most recently from f595d0a to 2080f75 Compare September 21, 2023 13:39
@Integralist Integralist force-pushed the integralist/sso branch 4 times, most recently from 9788044 to 7e24130 Compare October 12, 2023 16:31
@Integralist Integralist force-pushed the integralist/sso branch 3 times, most recently from 77fbbfc to 52d553d Compare October 18, 2023 12:17
@Integralist Integralist force-pushed the integralist/sso branch 2 times, most recently from 581ce35 to 4f5e3af Compare October 25, 2023 13:19
@Integralist Integralist force-pushed the integralist/sso branch 5 times, most recently from 467850a to 9de753a Compare November 9, 2023 14:57
@drhoden-fastly
Copy link

@Integralist what a huge lift! I've read through the PKCE workflow with great interest and the execution matches what I expected. I pulled the code down, ran the CLI locally and login with zero issues.

/cc: @phamann

@Integralist
Copy link
Collaborator Author

Just to leave an update to say this PR will be merged soon.

There are three ways to enable the SSO login flow for an existing CLI profile...

  1. fastly sso
  2. --enable-sso global flag
  3. FASTLY_USE_SSO=1 environment variable

Each of these approaches will switch the currently active profile from a traditional long-lived token to an SSO-based token (i.e. it'll replace the token in the user's config file with the short-lived token generated by the OAuth flow).

Any other profiles the user has in their config file (fastly config) will continue to work and will continue to have the long-lived token it already has assigned (i.e. none of the above options automatically switches all profiles to SSO, it has to be enabled on a profile-by-profile basis as users might not want all profiles switched from long-lived tokens).

@Integralist Integralist force-pushed the integralist/sso branch 2 times, most recently from a26ed51 to fcd54a8 Compare November 14, 2023 16:45
@Integralist Integralist merged commit 60e53e3 into main Nov 15, 2023
6 checks passed
@Integralist Integralist deleted the integralist/sso branch November 15, 2023 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants