Add Identity/Auth to template #300 #487

ahmedanwar100 · 2023-01-01T14:11:20Z

Jwt Authentication added with default Microsoft Identity

UserEndpoints folder created

Auth folder moved to Core project

ahmedanwar100 · 2023-01-01T14:14:24Z

Hi @ardalis, can you please review it

src/Clean.Architecture.Core/UserAggregate/User.cs

src/Clean.Architecture.Infrastructure/Data/AppDbContext.cs

ardalis

Overall looks great, thanks!

Only request is to don't put Identity stuff in Core (instead you can reference it using a UserId property on an appropriate entity, like in our case perhaps Contributor) and also keep IdentityDbContext separate from the regular/existing AppdbContext. Can you make those changes and I'll merge?

src/Clean.Architecture.Core/Services/Auth/Jwt/JwtService.cs

+    //We can use EncryptingCredentials options in SecurityTokenDescriptor and hence our JWT token will not be parsed by jwt.io site and it will only be decrypted only by our code.
+    //Hence you can secure your token and who can see it
+    var encryptionKey = Encoding.UTF8.GetBytes(_siteSetting.JwtSettings.EncryptKey); //must be 16 character
+    var encryptingCredentials = new EncryptingCredentials(new SymmetricSecurityKey(encryptionKey), SecurityAlgorithms.Aes128KW, SecurityAlgorithms.Aes128CbcHmacSha256);


src/Clean.Architecture.Infrastructure/StartupSetup.cs

+  {
+    return async context =>
+    {
+      var signInManager = context.HttpContext.RequestServices.GetRequiredService<SignInManager<User>>();


src/Clean.Architecture.Core/Services/Auth/Jwt/JwtService.cs

+    catch
+    {
+      return null;
+    }


src/Clean.Architecture.Infrastructure/StartupSetup.cs

+      //    context.Fail("This token has no security stamp");
+
+      //Get UserId and check if user exists in db
+      var userId = claimsIdentity.GetUserId<int>();


src/Clean.Architecture.Web/Endpoints/UserEndpoints/Login.cs

+  {
+    if (request is null)
+      ThrowError("Request body is empty");
+    var user = await _userManager.FindByEmailAsync(request.Email); //userName/email can be used to find a unique user


src/Clean.Architecture.Web/Endpoints/UserEndpoints/Login.cs

+      ThrowError("username or password is incorrect");
+
+    var jwt = await _jwtService.GenerateAsync(user);
+    user.LastLoginDate = DateTime.UtcNow;


src/Clean.Architecture.Web/Endpoints/UserEndpoints/SignUp.cs

+    //TODO:CHECK FOR USERNAME EMPTY OR NULL
+    var newUser = new User
+    {
+      Age = request.Age,


ahmedanwar100 · 2023-04-13T14:31:53Z

Can you give me time till sunday?

…

On Thu, 13 Apr 2023, 7:14 pm Steve Smith, ***@***.***> wrote: ***@***.**** requested changes on this pull request. Overall looks great, thanks! Only request is to don't put Identity stuff in Core (instead you can reference it using a UserId property on an appropriate entity, like in our case perhaps Contributor) and also keep IdentityDbContext separate from the regular/existing AppdbContext. Can you make those changes and I'll merge? — Reply to this email directly, view it on GitHub <#487 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AILYWTSUSE4B4XJVZ7VJTBTXBAC5DANCNFSM6AAAAAATOEIRPM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

ahmedanwar100 · 2023-04-13T14:33:26Z

I will try to complete it before sunday

…

On Thu, 13 Apr 2023, 7:31 pm Ahmed Anwer, ***@***.***> wrote: Can you give me time till sunday? On Thu, 13 Apr 2023, 7:14 pm Steve Smith, ***@***.***> wrote: > ***@***.**** requested changes on this pull request. > > Overall looks great, thanks! > > Only request is to don't put Identity stuff in Core (instead you can > reference it using a UserId property on an appropriate entity, like in our > case perhaps Contributor) and also keep IdentityDbContext separate from the > regular/existing AppdbContext. Can you make those changes and I'll merge? > > — > Reply to this email directly, view it on GitHub > <#487 (review)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AILYWTSUSE4B4XJVZ7VJTBTXBAC5DANCNFSM6AAAAAATOEIRPM> > . > You are receiving this because you authored the thread.Message ID: > ***@***.***> >

ardalis · 2023-04-13T14:48:10Z

No rush, thanks

ahmedanwar100 · 2023-04-13T14:48:57Z

Great, thanks!

…

On Thu, 13 Apr 2023, 7:48 pm Steve Smith, ***@***.***> wrote: No rush, thanks — Reply to this email directly, view it on GitHub <#487 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AILYWTXEQ3IDQCFZWGXRBF3XBAG3LANCNFSM6AAAAAATOEIRPM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

ahmedanwar100 · 2023-04-22T03:17:29Z

@ardalis can you please review it

DerekChasse · 2023-05-02T19:58:49Z

src/Clean.Architecture.Web/Endpoints/UserEndpoints/SignUp.cs

+      });
+      if (!addRoleResult.Succeeded) { ThrowValidationErrors(addRoleResult.Errors); }
+    }
+    else


This else does not look like it is necessary.

If I read correctly, you check to see if there is a role by that name already, if it doesn't you create it and only if it does exist do you then assign it to that user in this else clause.

Just removing the else around the inner code looks like it will solve the problem.

DerekChasse

Like I mentioned in one of my comments, I've essentially used the code in this pull request to implement Identity in my application which is based on the Clean Arch Template.

I can speak to the fact that the code written does in fact work, so there's that. 😄

For what it's worth, I moved the custom IdentityUser and IdentityRole implementations to the core project instead of the Infrastructure project. To do that, I needed to take a dependency on Microsoft.Extensions.Identity.Stores in the core project. In my eyes, that was a worth tradeoff allowing me to use the existing Event and Handler frameworks for these entities too. I did try to do things within the Infrastructure project but it was never as clearly defined as I wanted it to be.

@ardalis , I'll let you give the final thumbs up, but this is what I can share at least.

I suppose that this will lead to the inevitable ask for clarification on how to connect these two concepts. Tenancy and AppDatabase entity ownership etc...

DerekChasse · 2023-05-20T19:56:08Z

src/Clean.Architecture.Infrastructure/StartupSetup.cs

+/// </summary>
+internal class JwtJwtBearerHelperEvents
+{
+  public JwtBearerEvents GetJwtBeareEvents()


Nit: Missing 'r' in Bearer.

DerekChasse · 2023-05-20T19:57:28Z

src/Clean.Architecture.Infrastructure/StartupSetup.cs

+      OnChallenge = OnOnChallengeJwtBearerEvent()
+    };
+  }
+  private Func<AuthenticationFailedContext, Task> OnAuthenticationFailedJwtBearerEvent()


Nit: Not sure what the 'JwtBearerEvent' suffix adds to this, I'd drop it for simplification.

DerekChasse · 2023-05-20T19:59:13Z

src/Clean.Architecture.Infrastructure/Migrations/AppIdentityDbContextModelSnapshot.cs

@@ -0,0 +1,382 @@
+// <auto-generated />


Are these migrations necessary? For what it's worth, I've essentially used this pull request to bootstrap my applications and I do not use Migrations. Will these cause problems or confusion for new users of the template?

ahmedanwar100 · 2023-07-23T20:17:06Z

@ardalis, any plan to review it 😁

ardalis · 2023-09-01T16:16:03Z

My plan is still to pull this into the new template - just need to find some time.

ahmedanwar100 · 2023-09-01T16:53:03Z

Okay, then please find some time 🙂

TL\anwarahm added 4 commits December 31, 2022 00:40

Identity and JWT authentication setup added

6a22162

UserEndpoints folder created

Login Endpoint added

7202e7e

Auth folder moved to Core project

Helper class added to wrap JwtBearerEvents

c6e4233

appSettings set to defaut values

ea90fc0

ahmedanwar100 mentioned this pull request Jan 3, 2023

Add Identity/Auth to template #300

Open

Merge branch 'main' into add-identity-auth

3fe5844

ardalis reviewed Apr 13, 2023

View reviewed changes

src/Clean.Architecture.Core/UserAggregate/User.cs Outdated Show resolved Hide resolved

ardalis reviewed Apr 13, 2023

View reviewed changes

src/Clean.Architecture.Infrastructure/Data/AppDbContext.cs Outdated Show resolved Hide resolved

ardalis requested changes Apr 13, 2023

View reviewed changes

github-advanced-security bot found potential problems Apr 13, 2023

View reviewed changes

TL\anwarahm added 2 commits April 16, 2023 20:34

Identity: changes moved to Infrastructure

5d1a8ff

Code Rub: checks added and credential in apsettings removed

a1a83c7

ahmedanwar100 requested a review from ardalis April 16, 2023 16:06

CODE RUB: unsued variable removed

51e5ae1

DerekChasse suggested changes May 2, 2023

View reviewed changes

DerekChasse reviewed May 20, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Identity/Auth to template #300 #487

Add Identity/Auth to template #300 #487

ahmedanwar100 commented Jan 1, 2023

ahmedanwar100 commented Jan 1, 2023

ardalis left a comment

ahmedanwar100 commented Apr 13, 2023 via email

ahmedanwar100 commented Apr 13, 2023 via email

ardalis commented Apr 13, 2023

ahmedanwar100 commented Apr 13, 2023 via email

ahmedanwar100 commented Apr 22, 2023

DerekChasse May 2, 2023

DerekChasse left a comment

DerekChasse May 20, 2023

DerekChasse May 20, 2023

DerekChasse May 20, 2023 •

edited

ahmedanwar100 commented Jul 23, 2023

ardalis commented Sep 1, 2023

ahmedanwar100 commented Sep 1, 2023

Add Identity/Auth to template #300 #487

Are you sure you want to change the base?

Add Identity/Auth to template #300 #487

Conversation

ahmedanwar100 commented Jan 1, 2023

ahmedanwar100 commented Jan 1, 2023

ardalis left a comment

Choose a reason for hiding this comment

ahmedanwar100 commented Apr 13, 2023 via email

ahmedanwar100 commented Apr 13, 2023 via email

ardalis commented Apr 13, 2023

ahmedanwar100 commented Apr 13, 2023 via email

ahmedanwar100 commented Apr 22, 2023

DerekChasse May 2, 2023

Choose a reason for hiding this comment

DerekChasse left a comment

Choose a reason for hiding this comment

DerekChasse May 20, 2023

Choose a reason for hiding this comment

DerekChasse May 20, 2023

Choose a reason for hiding this comment

DerekChasse May 20, 2023 • edited

Choose a reason for hiding this comment

ahmedanwar100 commented Jul 23, 2023

ardalis commented Sep 1, 2023

ahmedanwar100 commented Sep 1, 2023

DerekChasse May 20, 2023 •

edited