Improve authentication and autorization

[alik90]

Improve authentication and autorization

[git-ari]

So I think we should used OAuth2 with the scopes instead of groups. https://oauth.net/2/scope/ I also think that public endpoints should need tokens. According to Katrina there will be no users, no adding add seems unnecessary, she says there will only be two different roles with different permissions or scopes.
As I said in the PR the solution Dotun did, looks well, but to me a bit overengineered. I think it can be simpler without as many new libraries, also missing logging and Swagger annotations, the exceptions aren't done as we had agreed previously.
And finally Swagger isnt working for the other endpoints because of the missing token.

[dotun]

@git-ari, the auth implementation was standard Spring Security, that takes care of both authentication and authorization.
I'm not clear on why you think OAuth2 scopes would be a better solution in this case. As I understand it, OAuth2 serves use cases where you want to expose user info to third-party clients without exposing credentials to said clients e.g. Login with google, github, linkedin etc.

Even with "only two different roles", there needs to be a standard way to manage authentication and authorization; this is provided as seen here and here.

When you say "overengineered", what exactly do you mean. The only libraries included for auth was for token generation and validation. Is logging mandatory? I assumed the Logging interface was to be used when logging is required in the implementing class. The missing Swagger annotations were due to the time constraints.

Regarding exceptions, I did ask if there was a way to use an exception with different messages. For example, token validation throws several different exceptions. defining a custom exception for every possible exception seems a bit much. Could we refactor the GlobalExceptionHandler to make use of exception messages contained within the exceptions instead of the .properties file? That way, we have a more scaleable exception handling strategy.

Swagger is working fine on my local machine. It even picks up the new endpoints (albeit without proper formatting).

Of course, formatting can be worked on further.

[dotun]

Regarding the token refresh time, I assume the concern is that the angular client is unable to make requests after the token has expired.

If that it is the concern, there is a solution to extend the token expiry described here, that I propose we implement.

[git-ari]

@git-ari, the auth implementation was standard Spring Security, that takes care of both authentication and authorization.
I'm not clear on why you think OAuth2 scopes would be a better solution in this case. As I understand it, OAuth2 serves use cases where you want to expose user info to third-party clients without exposing credentials to said clients e.g. Login with google, github, linkedin etc.

I'm sorry, I noticed I made a mistake. I wanted to say that I believe public endpoints should NOT need tokens, or users. I have researched the subject a bit more, and it may not apply with us, you are right. I personally have worked in a company where they used OAuth2 for managing the "authorities" even within the application, but maybe that made more sense back then since they used microservices architecture. I personally liked it and I think we could use it still, that it wouldn't make the application worse, but I understand if we don't do it.

When you say "overengineered", what exactly do you mean. The only libraries included for auth was for token generation and validation. Is logging mandatory? I assumed the Logging interface was to be used when logging is required in the implementing class. The missing Swagger annotations were due to the time constraints.

I mean for example all the auditing information, and like, we probably will only have three users, and two groups. So in my understanding of the requirements a user should only have one group, but a group can have multiple users, that way we don't need GROUP_MEMBERS and the authorizations I also don't think we need that, we only have two Roles, we can just check to which group the user belongs to removing the table GROUP_AUTHORITIES. We don't have a standard authentication requirements, no need to make to follow normal procedures and made it more complicated, following KISS pattern.
Logging should be mandatory as documentation both for the API as the code. When there are issues in the production environment we can only keep track of what happened by the logs, it helps detecting the issues and solve them as well as debugging, since you can set what you want to log for each Logging Level.

Regarding exceptions, I did ask if there was a way to use an exception with different messages. For example, token validation throws several different exceptions. defining a custom exception for every possible exception seems a bit much. Could we refactor the GlobalExceptionHandler to make use of exception messages contained within the exceptions instead of the .properties file? That way, we have a more scaleable exception handling strategy.

I believe its not up to us to define the messages that are presented on the front end. There should be different exceptions for different problems so that when we throw the exception back to the frontend it can look at its code and react differently to each. The message is there in the event someone just uses the API and it provides some extra information on what the problem was. In the way that it is implemented you can still define the message as you did but I think its not something that should be used often.

Swagger is working fine on my local machine. It even picks up the new endpoints (albeit without proper formatting).

You can use the endpoints for the autentication, but you cant use the others because there is no space to put the token. If its configured correctly you can use it with the token.