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.

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

Refactor guest routes #27

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Refactor guest routes #27

wants to merge 3 commits into from

Conversation

nekitbr
Copy link

@nekitbr nekitbr commented Apr 27, 2023

This PR intends to refactor the way whitelisted routes are unprotected.
After the merge, we can declare all whitelisted routes in one place and the filter will behave accordingly.

These issues were created before the original solution on the commit dfee1e5
closes #8
closes #12
closes #20

nekitbr
nekitbr commented May 6, 2023
View reviewed changes
Copy link
Author

@nekitbr nekitbr left a comment

Choose a reason for hiding this comment

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

The previous solution was bad and didn't user the antMatcher from the middleware RequestMatcher. This needs to be done because the String[] of routes is meant to be used there, thus we should abide by it's rules.

src/main/java/com/alibou/security/config/JwtAuthenticationFilter.java Outdated Show resolved Hide resolved
src/main/java/com/alibou/security/config/JwtAuthenticationFilter.java Show resolved Hide resolved
@nekitbr
Copy link
Author

nekitbr commented May 11, 2023

please if you could check this out @ali-bouali :)

@ali-bouali
Copy link
Owner

Hi @nekitbr ,
First off, thank you for your contribution and the effort you've put into this pull request. It's great to see your engagement with the project.
I really like the way you think about this to whitelist and improve the performance of the code.
I will accept this PR and showcase in the next youtube video before merging it.
I'm telling you this because I want always my code to be consistent with the videos, that is the only reason to postpone accepting your PR.

Also, I want to mention that your Github username will be on the video. I hope this is okay with you. Otherwise, please let me know so I can sensor it in the video

@nekitbr
Copy link
Author

nekitbr commented Jun 15, 2023

Also, I want to mention that your Github username will be on the video. I hope this is okay with you. Otherwise, please let me know so I can sensor it in the video

Thanks a lot for appreciating my contribution.

As for including my GitHub username in the video, I'm totally cool with it. Actually, I appreciate the recognition and exposure :))

Can't wait to see the video!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ERROR 403 ON AUTHENTICATION 403 forbidden error while Authentication permitAll() Not effective
2 participants
@nekitbr @ali-bouali