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

Use PasswordEncoder for ClientSecret storage #55

Closed
jricher opened this issue May 24, 2012 · 6 comments
Closed

Use PasswordEncoder for ClientSecret storage #55

jricher opened this issue May 24, 2012 · 6 comments

Comments

@jricher
Copy link
Member

jricher commented May 24, 2012

We should be using a PasswordEncoder to build and check client secrets in our client services:

org.springframework.security.authentication.encoding.PasswordEncoder

@nemonik
Copy link
Contributor

nemonik commented May 31, 2012

I can take this. Would you prefer sha or md5. It is entirely configurable as you know.

@nemonik nemonik closed this as completed May 31, 2012
@jricher jricher reopened this May 31, 2012
@jricher
Copy link
Member Author

jricher commented May 31, 2012

Either is fine so long as it works out of the box. This has to work against both the ClientDetailsEntity services and repositories (depending on where the right place to put these are) as well as any editing capabilities (such as the GUI and API that Jett's put together). There's also the issue that at some level we need to be able to get access to the plaintext client secret solely for the purpose of copying said secret to the client itself.

@nemonik
Copy link
Contributor

nemonik commented May 31, 2012

Added more to this response since responding via smart phone...

Typically, implementations of the PasswordEncoder, use the encode-method to create a hash, aka, a digest, of the password. Later, the DB stored password hash is compared against the hash calculated from the password the client sends for validation in the implemented isPasswordValid-method. So, using the Spring provided imps and best practice, we would not have the password stored just the hash so we would not have access to the password at a later date or in clear text in the DB, which should be avoided as I am sure you know and the reason to use the PasswordEncoder imps -- the practice this approach is trying to avoid. Am I correct in understanding by what you wrote in your last sentence that you'd want more than what is typical, and by that are you saying we'd need something different than what is the typically intended use of the PasswordEncoder interface. Thoughts?

Also, not sure why GIithub attributed me as closing the issue a few days ago..

@aanganes
Copy link
Contributor

aanganes commented Aug 7, 2012

Spring Security does not provide any two-way encryption methods for PasswordEncoder, only one-way hash functions. We could build one, but that gets complicated. I'm wondering if we can get around needing to have access to the plain-text password at all.

It sounds like we are generating the client's secret, and that needs to be passed on to the client after it is generated (and this is the ONLY time we need the plaintext). If so, then would it work to just notify the client about the secret (using some secure means) after it's generated, but before it is stored & encoded?

@ghost ghost assigned aanganes Aug 7, 2012
@jricher
Copy link
Member Author

jricher commented Aug 7, 2012

I've been thinking about this one more, too -- one approach would be to have the ability to show the password when it's generated and/or changed, but not once it's been stored. This seems to be what Ping does. For now we're going to assume full access to the client secrets and back-burner this.

@jricher
Copy link
Member Author

jricher commented Jun 15, 2017

The client secret needs to be reversible to support client_secret_jwt and the client management API, and keeping secrets plain doesn't reduce security much as they're not human-chosen values and won't be replicated.

@jricher jricher closed this as completed Jun 15, 2017
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

No branches or pull requests

3 participants