Skip to content

Add support for Client Registration Model and InMemory Client Repository #70

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
Merged
merged 1 commit into from
Apr 30, 2020
Merged

Add support for Client Registration Model and InMemory Client Repository #70

merged 1 commit into from
Apr 30, 2020

Conversation

anoopgarlapati
Copy link
Contributor

@anoopgarlapati anoopgarlapati commented Apr 25, 2020 •
edited
Loading

Fixes gh-40

@anoopgarlapati anoopgarlapati force-pushed the add-client-model-and-inmemory-repo branch 2 times, most recently from 7822f13 to 56bccaf Compare April 26, 2020 16:14
@anoopgarlapati anoopgarlapati force-pushed the add-client-model-and-inmemory-repo branch from 56bccaf to a3a75cd Compare April 26, 2020 18:30
@anoopgarlapati anoopgarlapati mentioned this pull request Apr 27, 2020
Merged
jgrandja
jgrandja requested changes Apr 28, 2020
View reviewed changes
Copy link
Collaborator

@jgrandja jgrandja left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This looks great @anoopgarlapati ! There are a few very minor changes. After you update please squash to one commit and we'll be ready to merge.

I just noticed that this is still in draft mode. Are there other changes you are planning?

@jgrandja jgrandja self-assigned this Apr 28, 2020
@jgrandja jgrandja added the type: enhancement A general enhancement label Apr 28, 2020
@jgrandja jgrandja added this to the 0.0.1 milestone Apr 28, 2020
@anoopgarlapati anoopgarlapati force-pushed the add-client-model-and-inmemory-repo branch from a3a75cd to 97af040 Compare April 28, 2020 10:45
@anoopgarlapati anoopgarlapati marked this pull request as ready for review April 28, 2020 10:54
@anoopgarlapati
Copy link
Contributor Author

@jgrandja I have updated the PR with the changes suggested.

@anoopgarlapati anoopgarlapati requested a review from jgrandja April 28, 2020 11:02
@anoopgarlapati anoopgarlapati force-pushed the add-client-model-and-inmemory-repo branch from 97af040 to 236fb80 Compare April 28, 2020 23:56
@jgrandja
Copy link
Collaborator

@anoopgarlapati Thanks for the quick response! I'll review this tomorrow.

I'm also thinking we should make this change to the Builder:

Instead of

public Builder clientAuthenticationMethods(ClientAuthenticationMethod... clientAuthenticationMethods)

public Builder clientAuthenticationMethods(Collection<ClientAuthenticationMethod> clientAuthenticationMethods)

Replace with

// additive
public Builder clientAuthenticationMethod(ClientAuthenticationMethod clientAuthenticationMethod)

public Builder clientAuthenticationMethods(Consumer<ClientAuthenticationMethod> clientAuthenticationMethodsConsumer)

This will make the Builder more fluent.

We would apply the same change to authorizationGrantTypes, redirectUris, and scopes.

What do you think?

@anoopgarlapati
Copy link
Contributor Author

@jgrandja I like the idea of additive fluent setters as well. I'll make the necessary changes.

@jgrandja
Copy link
Collaborator

Awesome @anoopgarlapati and great work!

@anoopgarlapati anoopgarlapati force-pushed the add-client-model-and-inmemory-repo branch from 236fb80 to 8700ff1 Compare April 29, 2020 01:58
@anoopgarlapati
Copy link
Contributor Author

@jgrandja I've updated the PR with changes to Builder.

@jgrandja jgrandja added the status: duplicate A duplicate of another issue label Apr 30, 2020
jgrandja added a commit that referenced this pull request Apr 30, 2020
@jgrandja
Polish gh-70
78f44c7
@jgrandja jgrandja merged commit 8700ff1 into spring-projects:master Apr 30, 2020
@jgrandja
Copy link
Collaborator

jgrandja commented Apr 30, 2020 •
edited
Loading

Thank you for the PR @anoopgarlapati. This is now in master! FYI, I added a polish commit with some minor updates.

And congrats as this is the very first implementation merged!

Let me know if you have free time to pick up another issue and I'll find you one.

@anoopgarlapati anoopgarlapati deleted the add-client-model-and-inmemory-repo branch May 1, 2020 03:51
@anoopgarlapati
Copy link
Contributor Author

@jgrandja Hey Joe, as discussed in meeting my team will reach out to you for further contribution on this project.
In the meantime, I can work on an issue if its available.

@jgrandja
Copy link
Collaborator

Sounds great @anoopgarlapati ! I will reach out to you offline.

doba16 pushed a commit to doba16/spring-authorization-server that referenced this pull request Apr 21, 2023
@jgrandja
Polish spring-projectsgh-70
226b259
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Captain-P-Goldfish Captain-P-Goldfish Captain-P-Goldfish requested changes

@jgrandja jgrandja jgrandja approved these changes

Assignees

@jgrandja jgrandja

Labels
status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Milestone
0.0.1
Development

Successfully merging this pull request may close these issues.

Implement Client Registration Model / Repository
3 participants
@anoopgarlapati @jgrandja @Captain-P-Goldfish