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

Jwtbr needs to merged in develop #238

Merged
merged 11 commits into from
Apr 22, 2020
Merged

Jwtbr needs to merged in develop #238

merged 11 commits into from
Apr 22, 2020

Conversation

vapadwal
Copy link
Member

@vapadwal vapadwal commented Apr 16, 2020

Implementation for #241

@hohwille hohwille changed the title Jwtbr needs to merged in develop WIP: Jwtbr needs to merged in develop Apr 16, 2020
@hohwille
Copy link
Member

I will have a closer look and do some final improvements myself. I am not yet sure when I will find a day of time to get this done.

@hohwille
Copy link
Member

We would really need an example demostrating this feature in action. I added some fundamental tests but do not have the time to complete the tests right now (already way to late).

  • How should JwtLoginFilter be used? Is this actually best practice? To send login+password via proprietary JSON IMHO seems to be against regular HTTP Authentication standards.
  • We need a full fledget sample or JUnit subsystem test with RANDOM_PORT testing the authentication via JWT as OAuth Bearer token with JwtAuthenticationFilter.
  • Dont we need a spring-starter for this feature?
  • Where is the documentation for all this stuff in the documentation folder?

@hohwille
Copy link
Member

@vapadwal how do we proceed? Should I merge this and you create another PR for improvements or will you create a PR for this PR before we merge?

@vapadwal
Copy link
Member Author

@vapadwal how do we proceed? Should I merge this and you create another PR for improvements or will you create a PR for this PR before we merge?

both plans sounds good, but it would be better if you merge this PR first

@vapadwal vapadwal changed the title WIP: Jwtbr needs to merged in develop Jwtbr needs to merged in develop Apr 22, 2020
Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

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

We still need some improvements for tests and documentation but we will do this step by step as this story has grown too big. So I will approve and merge...

@hohwille hohwille merged commit 67e7aed into develop Apr 22, 2020
@vapadwal vapadwal mentioned this pull request May 11, 2020
@hohwille hohwille deleted the jwtbr branch January 7, 2021 15:53
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.

None yet

2 participants