Implemented Jwt and Oauth2.0 Authentication

[rishabhrawat05]

Hi! @ajaynegi45
I have implemented all the functionality based on your issue #29

[ajaynegi45]

Hi @ ,

Thank you for your prompt work on this issue. I appreciate your dedication to the project.

I have reviewed your work, and I need some changes.

• Use constructor Injection not autowired.
• Where is login and logout controller.

Once again, thank you for your contribution! ❤️

[rishabhrawat05]

Hi! @ajaynegi45
Changes Implemented. Please review the changes

[rishabhrawat05]

Hey! @ajaynegi45
Are you still reviewing

[ajaynegi45]

Hey! @ajaynegi45 Are you still reviewing

Yes, It will take some time.

[Guhapriya01]

Hi @rishabhrawat05,

While running your code, I encountered an issue with the missing ClientRegistrationRepository bean, which prevents the app from running.

Could you please address this?

[rishabhrawat05]

Hi @rishabhrawat05,

While running your code, I encountered an issue with the missing ClientRegistrationRepository bean, which prevents the app from running.

Could you please address this?

Hey! @Guhapriya01
I will help you with the issue you are facing, In you application.properties file set Env:production and then set your client id, client secret in application-production.properties file then you might not face the issue
Thank you for reviewing

[Guhapriya01]

Thanks, @rishabhrawat05! I’ll go ahead and try setting the Env: production along with configuring the client-id and client-secret in the application-production.properties file. I’ll let you know once I’ve tested it out.

[Guhapriya01]

Hi @rishabhrawat05,

I’ve reviewed your changes, and they look good! However, I noticed a potential security issue with the /api/signup endpoint, as it allows anyone to create an account with any role, including admin.

Recommendations:

1. Set a default role of USER for new signups.
2. Create an endpoint for admins to promote users to higher roles (e.g., /api/admin/promote).

Additionally:

• The secret key in JwtAuthenticationHelper is hardcoded, consider using environment variables or config files for better security.
• Organizing the security package into sub-packages (e.g., com.libraryman_api.security.controllers and com.libraryman_api.security.services) can improve maintainability.

I’d love to hear your thoughts on these suggestions!

[rishabhrawat05]

Hey! @Guhapriya01
I will work on it

[rishabhrawat05]

Hey! @Guhapriya01
I have implemented all the changes that you requested

[Guhapriya01]

Hi @rishabhrawat05,

Thanks for implementing the changes! I’ll review them shortly and get back to you.

[Guhapriya01]

Thanks @rishabhrawat05 for the quick changes! Everything looks good.

I noticed that the /api/signup/admin and /api/signup/librarian endpoints still allow unrestricted access. Can we implement changes to restrict access to these endpoints?

[rishabhrawat05]

Thanks @rishabhrawat05 for the quick changes! Everything looks good.

I noticed that the /api/signup/admin and /api/signup/librarian endpoints still allow unrestricted access. Can we implement changes to restrict access to these endpoints?

Hey! @Guhapriya01
I understand that and worked on that but when working on that I realised that we have to give access of these api to user because we cant directly Authorize these endpoints with admin role and thus I cannot find any other way of authenticating these endpoint for only admin. If you have any suggestions please tell me I will change based on that

[Guhapriya01]

Hey @rishabhrawat05, I understand the challenge you're facing. If we assume there's already an existing admin, one solution is to manually assign the ADMIN role to a user via database access.

Once we have an admin, they can use secured API endpoints like /api/signup/admin or /api/signup/librarian to create other accounts with higher roles. Alternatively, we could create a single endpoint for role transfers that only admins can access, which would simplify the process and keep it secure.

This approach is common, and I’ve seen it work well in a previous project. Let me know if you think this would solve the issue!

[rishabhrawat05]

Hey @rishabhrawat05, I understand the challenge you're facing. If we assume there's already an existing admin, one solution is to manually assign the ADMIN role to a user via database access.

Once we have an admin, they can use secured API endpoints like /api/signup/admin or /api/signup/librarian to create other accounts with higher roles. Alternatively, we could create a single endpoint for role transfers that only admins can access, which would simplify the process and keep it secure.

This approach is common, and I’ve seen it work well in a previous project. Let me know if you think this would solve the issue!

@Guhapriya01 I understand your solution well but I think I have a better option for it why not we ask admin user a secret pass key( String pre defined or hardcoded key) when they signup and using that we can check if key is correct or not and then give them role admin and similarly for librarian
Let me know about your opinion on this approach

[rishabhrawat05]

Hey! @Guhapriya01
I have implemented the changes in the role-based authentication based on my approach can you please review and check if its suitable or not

[Guhapriya01]

Thanks for the updates! I’ll review your changes and get back to you soon.

[rishabhrawat05]

Hey! @ajaynegi45
I hope you are doing well. Just wanna know are you viewing all the changes that has been made and are you ok with it.

[Guhapriya01]

Hi @rishabhrawat05,

I noticed that the secret key is exposed in the URL as a path variable. Do you think this is secure?

[rishabhrawat05]

Hi @rishabhrawat05,

I noticed that the secret key is exposed in the URL as a path variable. Do you think this is secure

Yes as only admin can access this key and then he can signup to that api endpoint where he need that secret key

[Guhapriya01]

Yes as only admin can access this key and then he can signup to that api endpoint where he need that secret key

I understand, but since the secret key is in the URL, it could be visible to others. Doesn’t that create a security risk?

[rishabhrawat05]

Yes as only admin can access this key and then he can signup to that api endpoint where he need that secret key

I understand, but since the secret key is in the URL, it could be visible to others. Doesn’t that create a security risk?

So I should create a single in-memory account and that account can access signup which will register more admin user. But in this way the code is visible to anyone and anyone can view admin password so how how this will be secured

[rishabhrawat05]

Hey! @Guhapriya01
I have implemented the approach that you requested where you create admin user directly using database and then it can create other admin accounts. Please review this

[Guhapriya01]

Hi @rishabhrawat05,

Thank you for taking the time to implement the changes! I truly appreciate your effort in adapting the approach.

I've reviewed your changes, and I'm happy to say that I've merged your pull request. Everything looks great and aligns well with our project goals. Awesome job!

If you have more ideas or want to tackle other issues in the future, feel free to reach out. Your contributions are highly valued and greatly enhance the project.

Thanks again for your hard work! ❤️

[rishabhrawat05]

Hi @rishabhrawat05,

Thank you for taking the time to implement the changes! I truly appreciate your effort in adapting the approach.

I've reviewed your changes, and I'm happy to say that I've merged your pull request. Everything looks great and aligns well with our project goals. Awesome job!

If you have more ideas or want to tackle other issues in the future, feel free to reach out. Your contributions are highly valued and greatly enhance the project.

Thanks again for your hard work! ❤️

@Guhapriya01 thank you for teaching me something new about security this will elevate my skill and just one more thing please assign the labels that are assigned to this issue as this contribution will not be counted otherwise

[Guhapriya01]

@rishabhrawat05, I’m glad to hear that you found the security insights helpful! 😊

I've added the appropriate labels now. Looking forward to collaborating more!

[rishabhrawat05]

@rishabhrawat05, I’m glad to hear that you found the security insights helpful! 😊

I've added the appropriate labels now. Looking forward to collaborating more!

Thank you for constant support