feat(login): add flag to skip MAS-SSO login #477
Conversation
pkg/auth/login/login.go
Outdated
| @@ -35,20 +35,22 @@ type SSOConfig struct { | |||
| // Execute runs an Authorization Code flow login | |||
| // enabling the user to log in to SSO and MAS-SSO in succession | |||
| // https://tools.ietf.org/html/rfc6749#section-4.1 | |||
| func (a *AuthorizationCodeGrant) Execute(ctx context.Context, ssoCfg *SSOConfig, masSSOCfg *SSOConfig) (err error) { | |||
| func (a *AuthorizationCodeGrant) Execute(ctx context.Context, ssoCfg *SSOConfig, masSSOCfg *SSOConfig, skipMasSSOLogin bool) (err error) { | |||
There was a problem hiding this comment.
The smaller the number of arguments in a function the better. We already have masSSOCfg which can be updated with something generic like skipAuth: Boolean
| @@ -93,6 +94,7 @@ func NewLoginCmd(f *factory.Factory) *cobra.Command { | |||
| cmd.Flags().BoolVar(&opts.printURL, "print-sso-url", false, localizer.MustLocalizeFromID("login.flag.printSsoUrl")) | |||
| cmd.Flags().StringArrayVar(&opts.scopes, "scope", connection.DefaultScopes, localizer.MustLocalizeFromID("login.flag.scope")) | |||
| cmd.Flags().StringVarP(&opts.offlineToken, "token", "t", "", localizer.MustLocalizeFromID("login.flag.token")) | |||
| cmd.Flags().BoolVar(&opts.skipMasSSOLogin, "skip-mas-login", false, localizer.MustLocalizeFromID("login.flag.skipMasSSOLogin")) | |||
There was a problem hiding this comment.
| cmd.Flags().BoolVar(&opts.skipMasSSOLogin, "skip-mas-login", false, localizer.MustLocalizeFromID("login.flag.skipMasSSOLogin")) | |
| cmd.Flags().BoolVar(&opts.skipMasSSOLogin, "skip-mas-auth", false, localizer.MustLocalizeFromID("login.flag.skipMasSSOLogin")) |
There was a problem hiding this comment.
Dropped auth suffix as login might be more familiar to the end user, what do you think?
There was a problem hiding this comment.
I was thinking that we have the following flags: auth-url, mas-auth-url so it keeps with that pattern, i.e. it is obvious that skip-mas-auth means that mas-auth-url would not be used.
But I think you are correct, the command is called login, so skip-mas-login fits better with that.
| err = c.masKeycloakClient.Logout(ctx, c.clientID, "", c.masRealm, c.MASToken.RefreshToken) | ||
| if err != nil { | ||
| return &AuthError{err} | ||
| if c.MASToken.RefreshToken != "" { |
There was a problem hiding this comment.
Is it possible that the MAS Refresh token will not be an empty string? Are we clearing these values when the --skip-mas-login flag is passed?
There was a problem hiding this comment.
Thinking of it now, we might MAS refresh token wont be empty if user logins twice, first without passing the skip flag and then with the skip flag. We should clear them while passing --skip-mas-login, thanks :)
There was a problem hiding this comment.
Yep :) When you log out it clears them, but if your session expires they persist.
Description
Allow the user to skip logging in to MAS-SSO by passing a flag to the login command.
addresses #472
Verification Steps
--skip-mas-loginflag to the login command.You are now logged in as "<username>"Type of change
Checklist