Skip to content

Client secret double encoding issue when updating an existing registered client #389

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

Closed
ghost opened this issue Aug 9, 2021 · 4 comments
Labels
type: bug A general bug
Milestone

Comments

@ghost
Copy link

ghost commented Aug 9, 2021

When an existing client is updated via save(), the client-secret is double encoded.

The client_secret should be checked if its already encoded or not. If its already encoded, it should not be encoded again.

@ghost ghost added the type: bug A general bug label Aug 9, 2021
@jgrandja jgrandja added this to the 0.2.1 milestone Aug 9, 2021
@jgrandja jgrandja assigned ghost Aug 9, 2021
@ghost
Copy link
Author

ghost commented Aug 30, 2021

hi @jgrandja . I was on PTO for the past 2 weeks. I am back now, ready for work.

I have one question: is the upgradeEncoding method from the PasswordEncoder enough to check if the presented password should be encoded or not?
As far as I am aware, there's no straightforward way to check if a value is already encoded or not.

@jgrandja
Copy link
Collaborator

Hey @ovidiupopa91. I'll get back to you later this week. I'm just in the middle of preparing for our SpringOne presentation. Thanks.

@ghost
Copy link
Author

ghost commented Sep 1, 2021

Good luck with your presentation @jgrandja . I am sure it will be great 😃

@jgrandja
Copy link
Collaborator

jgrandja commented Sep 2, 2021

@ovidiupopa91

is the upgradeEncoding method from the PasswordEncoder enough to check if the presented password should be encoded or not?

No, this wouldn't be the method to call to determine if it's encoded.

Take a look at DelegatingPasswordEncoder.encode() as it formats the encoding with a prefix as such:

{bcrypt}$2a$10$x27xSzWVIdmj9bOTHk2STeQCGIiPZ0o2kOX1ewr.aFUOe2ohbnfxa

This indicates that it's bcrypt encoded.

I think we could simply check for { } to determine if it's already encoded.

FYI, I'm on PTO starting tomorrow and returning Sep 15.

Talk to you when I'm back.

ghost pushed a commit to ovidiupopa07/spring-authorization-server that referenced this issue Sep 7, 2021
ghost pushed a commit to ovidiupopa07/spring-authorization-server that referenced this issue Sep 7, 2021
ghost pushed a commit to ovidiupopa07/spring-authorization-server that referenced this issue Sep 7, 2021
ghost pushed a commit to ovidiupopa07/spring-authorization-server that referenced this issue Sep 8, 2021
ghost pushed a commit to ovidiupopa07/spring-authorization-server that referenced this issue Sep 8, 2021
ghost pushed a commit to ovidiupopa07/spring-authorization-server that referenced this issue Sep 8, 2021
ghost pushed a commit to ovidiupopa07/spring-authorization-server that referenced this issue Sep 23, 2021
This might have to be revisited at a later point, but to check if a value is encoded or not is quite tricky. The decision was to remove client_secret and client_secret_expires_at from the update statement

Closes spring-projectsgh-389
ghost pushed a commit to ovidiupopa07/spring-authorization-server that referenced this issue Sep 23, 2021
This might have to be revisited at a later point, but to check if a value is encoded or not is quite tricky. The decision was to remove client_secret and client_secret_expires_at from the update statement

Closes spring-projectsgh-389
doba16 pushed a commit to doba16/spring-authorization-server that referenced this issue Apr 21, 2023
This might have to be revisited at a later point, but to check if a value is encoded or not is quite tricky. The decision was to remove client_secret and client_secret_expires_at from the update statement

Closes spring-projectsgh-389
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
1 participant