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

[Bug]: First fragment parameter is ignored #1346

Closed
Samuel-B-D opened this issue Jan 6, 2022 · 3 comments · Fixed by #1359
Closed

[Bug]: First fragment parameter is ignored #1346

Samuel-B-D opened this issue Jan 6, 2022 · 3 comments · Fixed by #1359
Assignees

Comments

@Samuel-B-D
Copy link
Contributor

What Version of the libraray are you using?
I am using 12.0.3, but the error in the implementation of getUrlParameter is the same in the 13 branch and the main branch.

Describe the bug
getUrlParameter in the UrlService does not parse properly oauth request with a response_type of fragment.
( https://github.com/damienbod/angular-auth-oidc-client/blob/main/projects/angular-auth-oidc-client/src/lib/utils/url/url.service.ts#L26 )
specifically, it will skip the first parameter, so if the oauth provider redirect to https://mysite.com/#code=abc&state=123&... , the library will return a no code in url error.
This produce a looping behavior that will eventually resolve only when the first fragment is not required in the case of an openid provider returning fragments in an indetermined order, or will never work if the first fragment is always "code" or "state".

To Reproduce
Receive an oauth response in response_mode = 'fragment' with the first fragment being a required parameter for the oauth flow.

Expected behavior
The implementation should properly read the first fragment parameter.
https://github.com/damienbod/angular-auth-oidc-client/blob/main/projects/angular-auth-oidc-client/src/lib/utils/url/url.service.ts#L26

@FabianGosebrink
Copy link
Collaborator

Hey, thanks. Can you open a PR and write a failing test for it on a branch, then we can fix it accordingly. The method is public so it should be easy to test it. Thanks!

@Samuel-B-D
Copy link
Contributor Author

Sorry for the late reply. I will do that once I get the time available to do so 😅

@Samuel-B-D
Copy link
Contributor Author

There! Made the fix in addition to the test since it was trivial.
I also took an example url straight from the RFC for good measure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants