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

client_secret is optional in all grant types #12

Merged
merged 4 commits into from
May 12, 2017
Merged

client_secret is optional in all grant types #12

merged 4 commits into from
May 12, 2017

Conversation

Tobion
Copy link

@Tobion Tobion commented Apr 4, 2017

The client ID is considered public information, and is used to build login URLs, or included in Javascript source code on a page. The client secret must be kept confidential. If a deployed app cannot keep the secret confidential, such as single-page Javascript apps or native apps, then the secret is not used, and ideally the service shouldn't issue a secret to these types of apps in the first place.

https://aaronparecki.com/oauth-2-simplified/

When there is no client_secret you also must not send one (in our case pivotal cloudfoundry oauth). But the current validation makes this impossible.

@mweibel
Copy link

mweibel commented Apr 19, 2017

@dm ping? :)

@dm
Copy link

dm commented Apr 19, 2017

Hi @mweibel @Tobion, I've since left Sainsbury's, but I'll try to ping someone there to look at this.
Cheers,
Dan

@pharman
Copy link

pharman commented Apr 19, 2017

Hi @Tobion - could you add a note on the readme about this and add a simple test case?

@Tobion
Copy link
Author

Tobion commented May 4, 2017

I updated the readme and the tests which fixes #8 and #10 as well.

@Tobion
Copy link
Author

Tobion commented May 12, 2017

@pharman please merge

@pharman
Copy link

pharman commented May 12, 2017

Thanks @Tobion

@pharman pharman merged commit 68eb4e4 into Sainsburys:master May 12, 2017
@pharman
Copy link

pharman commented May 12, 2017

@Tobion Tobion deleted the fix-optional-secret branch May 12, 2017 15:46
@Tobion
Copy link
Author

Tobion commented May 12, 2017

Thank you

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

Successfully merging this pull request may close these issues.

4 participants