Google auth #103
Google auth #103
Conversation
There is added a new use case for login/logout. The database is extended by a user model where are situated claims from google API for auth.
There was a problem hiding this comment.
Hey, @TerezaSkutova overall you did a pretty good job, there are just a couple of small tweaks we need to do before it will be ready for merge. 😊
|
|
||
| public interface IUserRepository : IGenericRepository<UserModel, IEntity> | ||
| { | ||
| Task<UserModel> GetUserByGoogleId(string id); |
There was a problem hiding this comment.
Optional: provider Id would be optional in case of future extensibility. Coupling names to a specific platform or tool should be avoided if possible.
There was a problem hiding this comment.
it applies across the whole solution.
| claim.Value | ||
| }).ToDictionary(claim => claim.Type.Substring(claim.Type.LastIndexOf("/") + 1), claim => claim.Value); | ||
|
|
||
| var input = new EnsureCreatedUserInput |
There was a problem hiding this comment.
With this solution, the attributes are unprotected from malicious or malformed input.
You can extend input to provide some sort of domain validation on important attributes.
Or instead of casting values to the dictionary cast them to objects so, we can use data attributes.
add validation also to others attributes.
| "Google": { | ||
| "ClientID": "172955704257-ug7p7mbjrd7ogkegvjfldnofal57q11b.apps.googleusercontent.com", | ||
| "ClientSecret": "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" | ||
| } |
There was a problem hiding this comment.
I don't think this should be here 🙈 we can fill value in the pipeline if needed.
| } | ||
|
|
||
| [HttpGet(Name = "Login")] | ||
| public async Task Login() |
There was a problem hiding this comment.
There are 2 methods in one controller could we split it? 🤔
There was a problem hiding this comment.
It is closely related to Login method. Does it really make sense?
| public UserRepository(MyDbContext context, IMapper mapper) : base(context, mapper) | ||
| { | ||
| } | ||
| public async Task<UserModel> GetUserByGoogleId(string googleId) |
|
|
||
| if (model is null) | ||
| { | ||
| _outputPort.Invalid(); |
There was a problem hiding this comment.
Comment: I noticed this earlier. You can pick new names for methods in the output port. Sometimes invalid makes sense but could be Failed, NotFound, Corrupted, anything that comes to your mind for a given context. 😊
| { | ||
| var user = await _userRepository.GetUserByGoogleId(input.GoogleId); | ||
|
|
||
| if(user is null) |
There was a problem hiding this comment.
could we use the return early principle in this case? 🤔
There is added a new use case for login/logout. The database is extended by a user model where are situated claims from google API for auth.
….com:3PillarGlobal-Czechia/interview-app-api into feature/74-spike-create-poc-for-google-oauth
|
Kudos, SonarCloud Quality Gate passed! |
Summary
The authentication is resolved by Google API on Google Cloud and it is allowing accounts only from our domain (@3pillarglobal.com). If you try to login with another email address, you will get an error 403.
There is created a user record on database for each new logged user. The model contains:
Types of changes
Testing
I have been testing this feature on my account.