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

OIDC Alternate Path Bug #17661

Merged
merged 6 commits into from
Oct 26, 2022
Merged

OIDC Alternate Path Bug #17661

merged 6 commits into from
Oct 26, 2022

Conversation

zofskeez
Copy link
Contributor

@zofskeez zofskeez commented Oct 26, 2022

This PR addresses issues with error handling within the OIDC login workflow and fixes a bug with the inputted role not being retained when attempting to login at an alternate mount path. This was brought up in issue #14671 where it was also pointed out that logging in with the jwt auth method using the mount tabs was returning an error which has also been fixed.

Alternate mount path issue as observed in 1.12.0:
oidc-role-bug

After clicking the sign in button numerous times it appears that nothing happens, when in reality the role is not passed in the request for the auth_url and the check for that returns out of the function without any communication to the user.

After updates:
oidc-role-bug-fix

The foo role doesn't exist for the foo-oidc path and now an error is displayed. Next, the bar role does not have a redirect_uri for the path foo-oidc. Once the auth_url is available we can now login as expected.

… bug where role wasn't being retained when using alternate oidc mount path at login
Copy link
Contributor

@Monkeychip Monkeychip left a comment

Choose a reason for hiding this comment

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

Nothing blocking, nice work! A couple of non-blocking comments.

{{#if (or (this.error) (and this.waitingForOktaNumberChallenge (not this.cancelAuthForOktaNumberChallenge)))}}
{{#if (and this.waitingForOktaNumberChallenge (not this.cancelAuthForOktaNumberChallenge))}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed while testing error handling in AuthJwt that this was catching all errors and also throwing an error of it's own since it was trying to lookup this.error as a helper. It doesn't look like the error prop is relevant for the condition so I removed it.

@zofskeez zofskeez merged commit aa94835 into main Oct 26, 2022
@zofskeez zofskeez deleted the ui/VAULT-9464/oidc-login-bug branch October 26, 2022 21:39
zofskeez added a commit that referenced this pull request Oct 27, 2022
* adds error handling to auth-jwt component for missing roles and fixes bug where role wasn't being retained when using alternate oidc mount path at login

* fixes jwt login bug from auth mount tabs and adds test

* updates okta-number-challenge success value to arg in template

* adds changelog entry

* fixes issues logging in manually with jwt

* reverts mistaken change
zofskeez added a commit that referenced this pull request Oct 27, 2022
* adds error handling to auth-jwt component for missing roles and fixes bug where role wasn't being retained when using alternate oidc mount path at login

* fixes jwt login bug from auth mount tabs and adds test

* updates okta-number-challenge success value to arg in template

* adds changelog entry

* fixes issues logging in manually with jwt

* reverts mistaken change
zofskeez added a commit that referenced this pull request Oct 27, 2022
* adds error handling to auth-jwt component for missing roles and fixes bug where role wasn't being retained when using alternate oidc mount path at login

* fixes jwt login bug from auth mount tabs and adds test

* updates okta-number-challenge success value to arg in template

* adds changelog entry

* fixes issues logging in manually with jwt

* reverts mistaken change
zofskeez added a commit that referenced this pull request Oct 27, 2022
* adds error handling to auth-jwt component for missing roles and fixes bug where role wasn't being retained when using alternate oidc mount path at login

* fixes jwt login bug from auth mount tabs and adds test

* updates okta-number-challenge success value to arg in template

* adds changelog entry

* fixes issues logging in manually with jwt

* reverts mistaken change
@zofskeez zofskeez modified the milestones: 1.13.0-rc1, 1.10.8 Oct 27, 2022
zofskeez added a commit that referenced this pull request Oct 27, 2022
* adds error handling to auth-jwt component for missing roles and fixes bug where role wasn't being retained when using alternate oidc mount path at login

* fixes jwt login bug from auth mount tabs and adds test

* updates okta-number-challenge success value to arg in template

* adds changelog entry

* fixes issues logging in manually with jwt

* reverts mistaken change
@zofskeez zofskeez added the bug Used to indicate a potential bug label Oct 27, 2022
zofskeez added a commit that referenced this pull request Oct 27, 2022
* adds error handling to auth-jwt component for missing roles and fixes bug where role wasn't being retained when using alternate oidc mount path at login

* fixes jwt login bug from auth mount tabs and adds test

* updates okta-number-challenge success value to arg in template

* adds changelog entry

* fixes issues logging in manually with jwt

* reverts mistaken change
zofskeez added a commit that referenced this pull request Oct 27, 2022
* adds error handling to auth-jwt component for missing roles and fixes bug where role wasn't being retained when using alternate oidc mount path at login

* fixes jwt login bug from auth mount tabs and adds test

* updates okta-number-challenge success value to arg in template

* adds changelog entry

* fixes issues logging in manually with jwt

* reverts mistaken change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants