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

Jump to bottom

Fixing login request loop when PKCE is mandatory on Asgardeo console but disabled on the SDK #15

Merged
merged 3 commits into from
Jul 14, 2022
Merged

Fixing login request loop when PKCE is mandatory on Asgardeo console but disabled on the SDK #15

merged 3 commits into from
Jul 14, 2022

Conversation

nipunsampath
Copy link
Contributor

Purpose

Fixes #14. The reason for this issue is when sign-in fails with an error (because PKCE is mandatory in this case), Asgardeo redirects the application to the specified login URL. As a result, a login request is sent again, and Asgardeo redirects the application back to the login URL with an error. This process results in a login request loop.

Approach

In order to fix the issue, the signIn() method now checks whether there is an error query parameter in the login URL before sending a login request. If there is an error query parameter, AsgardeoAuthException is thrown.

@nipunsampath nipunsampath requested a review from thivi July 11, 2022 05:30
thivi
thivi approved these changes Jul 11, 2022
View reviewed changes
lib/src/utils/express-utils.ts Outdated Show resolved Hide resolved
@nipunsampath nipunsampath force-pushed the login-loop-with-pkce-disabled-fix branch from 1e5e00a to a5d4681 Compare July 11, 2022 10:08
@nipunsampath nipunsampath requested a review from thivi July 11, 2022 10:44
thivi
thivi approved these changes Jul 14, 2022
View reviewed changes
lib/src/utils/express-utils.ts Show resolved Hide resolved
@thivi thivi merged commit 24e0589 into asgardeo:main Jul 14, 2022
ThaminduDilshan
ThaminduDilshan reviewed Feb 10, 2023
View reviewed changes
lib/src/utils/express-utils.ts
* @returns {boolean} - True if the URL contains an error.
*/
public static hasErrorInURL(url: string): boolean {
const AUTH_CODE_REGEXP: RegExp = /[?&]error=[^&]+/;
Copy link

Choose a reason for hiding this comment

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

Shall we extract this constant to a class variable?

@nipunsampath nipunsampath mentioned this pull request Feb 23, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ThaminduDilshan ThaminduDilshan ThaminduDilshan left review comments

@thivi thivi thivi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Login request loop when PKCE is mandatory on Asgardeo console but disabled on the SDK
3 participants
@nipunsampath @thivi @ThaminduDilshan