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.

Sign up for GitHub

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(login): add flag to skip MAS-SSO login #477

Merged
merged 3 commits into from
Mar 24, 2021
Merged

feat(login): add flag to skip MAS-SSO login #477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b7d2fe9
feat(login): add flag to skip MAS-SSO login
rkpattnaik780 Mar 23, 2021
6cfb1a8
fix: skip mas sso logout if MAS refresh token is empty
rkpattnaik780 Mar 23, 2021
59e9c9e
fix: handle double logins
rkpattnaik780 Mar 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/rhoas/pkged.go
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions docs/commands/rhoas_login.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ $ rhoas login --print-sso-url
--mas-auth-url string The URL of the MAS-SSO Authentication server. (default "https://keycloak-edge-redhat-rhoam-user-sso.apps.mas-sso-stage.1gzl.s1.devshift.org/auth/realms/mas-sso-staging")
--print-sso-url Prints the console login URL, which you can use to log in to RHOAS from a different web browser. This is useful if you need to log in with different credentials than the credentials you used in your default web browser.
--scope stringArray Override the default OpenID scope. To specify multiple scopes, use a separate --scope for each scope. (default [openid])
--skip-mas-login Skip logging in to MAS-SSO
-t, --token string Allows you to log in using an offline token, which can be obtained at https://cloud.redhat.com/openshift/token.
....

Expand Down
4 changes: 4 additions & 0 deletions locales/cmd/login/active.en.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ one = "Prints the console login URL, which you can use to log in to RHOAS from a
description = 'Description for the --scope flag'
one = 'Override the default OpenID scope. To specify multiple scopes, use a separate --scope for each scope.'

[login.flag.skipMasSSOLogin]
description = 'Description for the --skip-mas-login flag'
one = 'Skip logging in to MAS-SSO'

[login.redirectPage.title]
description = 'Title for the RHOAS login redirect webpage'
one = 'Welcome to RHOAS'
Expand Down
24 changes: 21 additions & 3 deletions pkg/auth/login/login.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,40 @@ type AuthorizationCodeGrant struct {
type SSOConfig struct {
AuthURL string
RedirectPath string
SkipAuth bool
}

// 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) error {
// log in to SSO
a.Logger.Info(localizer.MustLocalizeFromID("login.log.info.loggingIn"))
if err = a.loginSSO(ctx, ssoCfg); err != nil {
if err := a.loginSSO(ctx, ssoCfg); err != nil {
return err
}
a.Logger.Info(localizer.MustLocalizeFromID("login.log.info.loggedIn"))

if masSSOCfg.SkipAuth {

cfg, err := a.Config.Load()
if err != nil {
return err
}

cfg.MasAccessToken = ""
cfg.MasRefreshToken = ""

if err = a.Config.Save(cfg); err != nil {
return err
}

return nil
}

a.Logger.Info(localizer.MustLocalizeFromID("login.log.info.loggingInMAS"))
// log in to MAS-SSO
if err = a.loginMAS(ctx, masSSOCfg); err != nil {
if err := a.loginMAS(ctx, masSSOCfg); err != nil {
return err
}
a.Logger.Info(localizer.MustLocalizeFromID("login.log.info.loggedInMAS"))
Expand Down
3 changes: 3 additions & 0 deletions pkg/cmd/login/login.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ type Options struct {
insecureSkipTLSVerify bool
printURL bool
offlineToken string
skipMasSSOLogin bool
}

// NewLoginCmd gets the command that's log the user in
Expand Down Expand Up @@ -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"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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"))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropped auth suffix as login might be more familiar to the end user, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


return cmd
}
Expand Down Expand Up @@ -146,6 +148,7 @@ func runLogin(opts *Options) (err error) {
masSsoCfg := &login.SSOConfig{
AuthURL: opts.masAuthURL,
RedirectPath: "mas-sso-callback",
SkipAuth: opts.skipMasSSOLogin,
}

if err = loginExec.Execute(context.Background(), ssoCfg, masSsoCfg); err != nil {
Expand Down
8 changes: 5 additions & 3 deletions pkg/connection/keycloak_connection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,11 @@ func (c *KeycloakConnection) Logout(ctx context.Context) (err error) {
return &AuthError{err}
}

err = c.masKeycloakClient.Logout(ctx, c.clientID, "", c.masRealm, c.MASToken.RefreshToken)
if err != nil {
return &AuthError{err}
if c.MASToken.RefreshToken != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep :) When you log out it clears them, but if your session expires they persist.

err = c.masKeycloakClient.Logout(ctx, c.clientID, "", c.masRealm, c.MASToken.RefreshToken)
if err != nil {
return &AuthError{err}
}
}

c.Token.AccessToken = ""
Expand Down