feature/218/accessibility-logIn-using-google-auth#244
Conversation
WalkthroughAdds Google OAuth login end-to-end: new DTO, service and interface, MediatR query/handler, controller endpoints, DI registration and cookie/Google auth configuration, package references across projects, dotnet tool config, and a serviceDependencies entry; removes two unused Program.cs usings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Browser as User
participant AC as AuthController
participant ASP as ASP.NET Auth (Cookie + Google)
participant GServ as GoogleService
participant Med as MediatR (GoogleLoginHandler)
participant UM as UserManager<User>
participant JWT as IJwtTokenService
User->>AC: GET /api/auth/google
AC-->>ASP: Challenge("Google") (redirect /api/auth/google-callback)
ASP-->>User: Redirect to Google OAuth
User-->>ASP: Google callback -> redirect /api/auth/google-callback
User->>AC: GET /api/auth/google-callback
AC->>Med: Send(GoogleLoginQuery)
Med->>GServ: GetGoogleUserInfoAsync()
GServ->>ASP: AuthenticateAsync("Google")
ASP-->>GServ: ClaimsPrincipal (email, name, sub)
GServ-->>Med: GoogleUserInfoDTO
Med->>UM: FindByEmailAsync / FindByLoginAsync
alt existing user
Med->>UM: Update names if changed
else new user
Med->>UM: CreateAsync -> AddToRoleAsync -> AddLoginAsync(provider="Google")
end
Med->>JWT: GenerateTokenAsync(user)
JWT-->>Med: LoginResultDTO
Med-->>AC: Result<LoginResultDTO>
AC-->>User: 200 OK (token) / 400 BadRequest on errors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (6)
Streetcode/Streetcode.BLL/Interfaces/Google/IGoogleService.cs (1)
6-6: Consider adding CancellationToken.Surface cancellation to callers in web flows.
- Task<GoogleUserInfoDTO> GetGoogleUserInfoAsync(); + Task<GoogleUserInfoDTO> GetGoogleUserInfoAsync(CancellationToken cancellationToken = default);Streetcode/Streetcode.BLL/DTO/Users/GoogleLogin/GoogleUserInfoDTO.cs (1)
5-8: Nullability: mark properties as required (or nullable).Avoid nullability warnings and accidental nulls.
- public string Email { get; set; } - public string GivenName { get; set; } - public string FamilyName { get; set; } - public string Subject { get; set; } + public required string Email { get; set; } + public required string GivenName { get; set; } + public required string FamilyName { get; set; } + public required string Subject { get; set; }Streetcode/Streetcode.BLL/Services/Google/GoogleService.cs (1)
1-6: Import cookie defaults for readability (optional).using System.Security.Claims; using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Http;Streetcode/Streetcode.BLL/MediatR/Auth/GoogleLogin/GoogleLoginHandler.cs (2)
46-49: Use ToLowerInvariant for culture-invariant usernames.- UserName = userInfo.Email.Split("@")[0].ToLower() + UserName = userInfo.Email.Split('@')[0].ToLowerInvariant()
43-51: Potential username collision.Splitting email local-part can produce duplicates; consider ensuring uniqueness (e.g., suffix if taken).
Would you like a helper that probes UserManager for availability and generates a unique username?
Streetcode/Streetcode.WebApi/Streetcode.WebApi.csproj (1)
22-22: Align auth package versions across projectsMicrosoft.AspNetCore.Authentication.Google is 8.0.20 while JwtBearer/Identity are 8.0.19 — align patch versions (prefer 8.0.20).
Files to update:
- Streetcode/Streetcode.WebApi/Streetcode.WebApi.csproj — Google 8.0.20; Microsoft.AspNetCore.Authentication.JwtBearer 8.0.19; Microsoft.AspNetCore.Identity.EntityFrameworkCore 8.0.19
- Streetcode/Streetcode.DAL/Streetcode.DAL.csproj — Google 8.0.20; Microsoft.AspNetCore.Identity.EntityFrameworkCore 8.0.19
Other projects referencing Google 8.0.20: build/_build.csproj, Streetcode/DbUpdate/DbUpdate.csproj, Streetcode/Streetcode.BLL/Streetcode.BLL.csproj, Streetcode/Streetcode.XIntegrationTest/Streetcode.XIntegrationTest.csproj, Streetcode/Streetcode.XUnitTest/Streetcode.XUnitTest.csproj
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
Streetcode/DbUpdate/DbUpdate.csproj(1 hunks)Streetcode/Streetcode.BLL/DTO/Users/GoogleLogin/GoogleUserInfoDTO.cs(1 hunks)Streetcode/Streetcode.BLL/Interfaces/Google/IGoogleService.cs(1 hunks)Streetcode/Streetcode.BLL/MediatR/Auth/GoogleLogin/GoogleLoginHandler.cs(1 hunks)Streetcode/Streetcode.BLL/MediatR/Auth/GoogleLogin/GoogleLoginQuery.cs(1 hunks)Streetcode/Streetcode.BLL/Services/Google/GoogleService.cs(1 hunks)Streetcode/Streetcode.BLL/Streetcode.BLL.csproj(1 hunks)Streetcode/Streetcode.DAL/Streetcode.DAL.csproj(1 hunks)Streetcode/Streetcode.WebApi/.config/dotnet-tools.json(1 hunks)Streetcode/Streetcode.WebApi/Controllers/Auth/AuthController.cs(2 hunks)Streetcode/Streetcode.WebApi/Extensions/ServiceCollectionExtensions.cs(4 hunks)Streetcode/Streetcode.WebApi/Program.cs(0 hunks)Streetcode/Streetcode.WebApi/Properties/serviceDependencies.json(1 hunks)Streetcode/Streetcode.WebApi/Streetcode.WebApi.csproj(1 hunks)Streetcode/Streetcode.XIntegrationTest/Streetcode.XIntegrationTest.csproj(1 hunks)Streetcode/Streetcode.XUnitTest/Streetcode.XUnitTest.csproj(1 hunks)build/_build.csproj(1 hunks)
💤 Files with no reviewable changes (1)
- Streetcode/Streetcode.WebApi/Program.cs
🧰 Additional context used
🧬 Code graph analysis (6)
Streetcode/Streetcode.BLL/Services/Google/GoogleService.cs (3)
Streetcode/Streetcode.BLL/Interfaces/Google/IGoogleService.cs (1)
Task(6-6)Streetcode/Streetcode.BLL/MediatR/Auth/GoogleLogin/GoogleLoginHandler.cs (2)
Task(34-96)Task(98-135)Streetcode/Streetcode.BLL/DTO/Users/GoogleLogin/GoogleUserInfoDTO.cs (1)
GoogleUserInfoDTO(3-9)
Streetcode/Streetcode.WebApi/Controllers/Auth/AuthController.cs (3)
Streetcode/Streetcode.BLL/Interfaces/Google/IGoogleService.cs (1)
Task(6-6)Streetcode/Streetcode.BLL/MediatR/Auth/GoogleLogin/GoogleLoginHandler.cs (2)
Task(34-96)Task(98-135)Streetcode/Streetcode.BLL/Services/Google/GoogleService.cs (1)
Task(20-48)
Streetcode/Streetcode.WebApi/Extensions/ServiceCollectionExtensions.cs (1)
Streetcode/Streetcode.BLL/Services/Google/GoogleService.cs (2)
GoogleService(9-49)GoogleService(15-18)
Streetcode/Streetcode.BLL/MediatR/Auth/GoogleLogin/GoogleLoginHandler.cs (4)
Streetcode/Streetcode.BLL/DTO/Users/LoginResultDTO.cs (1)
LoginResultDTO(9-14)Streetcode/Streetcode.BLL/Interfaces/Google/IGoogleService.cs (1)
Task(6-6)Streetcode/Streetcode.BLL/Services/Google/GoogleService.cs (1)
Task(20-48)Streetcode/Streetcode.BLL/Services/Logging/LoggerService.cs (1)
LogError(35-40)
Streetcode/Streetcode.BLL/Interfaces/Google/IGoogleService.cs (1)
Streetcode/Streetcode.BLL/DTO/Users/GoogleLogin/GoogleUserInfoDTO.cs (1)
GoogleUserInfoDTO(3-9)
Streetcode/Streetcode.BLL/MediatR/Auth/GoogleLogin/GoogleLoginQuery.cs (1)
Streetcode/Streetcode.BLL/DTO/Users/LoginResultDTO.cs (1)
LoginResultDTO(9-14)
🔇 Additional comments (7)
Streetcode/Streetcode.WebApi/.config/dotnet-tools.json (1)
12-18: msidentity tool: OK.No action needed.
Streetcode/Streetcode.XIntegrationTest/Streetcode.XIntegrationTest.csproj (1)
15-15: Remove unused Microsoft.AspNetCore.Authentication.Google from XIntegrationTestNo references to AddGoogle/GoogleDefaults/GoogleOptions or Microsoft.AspNetCore.Authentication.Google were found in Streetcode/Streetcode.XIntegrationTest; remove the PackageReference at Streetcode/Streetcode.XIntegrationTest/Streetcode.XIntegrationTest.csproj (line 15) and run build/tests to verify.
Streetcode/Streetcode.XUnitTest/Streetcode.XUnitTest.csproj (1)
24-24: Remove unused Google auth PackageReference from XUnit test projectNo references to Google auth APIs were found in Streetcode/Streetcode.XUnitTest sources. Remove the PackageReference.
Location: Streetcode/Streetcode.XUnitTest/Streetcode.XUnitTest.csproj:24
- <PackageReference Include="Microsoft.AspNetCore.Authentication.Google" Version="8.0.20" />Streetcode/Streetcode.WebApi/Properties/serviceDependencies.json (1)
1-9: LGTM — ensure DefaultConnection is populated at runtimeappsettings.json contains "ConnectionStrings:DefaultConnection" but its value is empty; the project enables user-secrets and docker-compose supplies ConnectionStrings__DefaultConnection. Provide a non-empty connection string via user-secrets or environment before running.
- Streetcode/Streetcode.WebApi/appsettings.json (DefaultConnection = "")
- Streetcode/Streetcode.WebApi/Properties/serviceDependencies.json (uses ConnectionStrings:DefaultConnection)
- Streetcode/Streetcode.WebApi/Streetcode.WebApi.csproj (UserSecretsId)
Streetcode/Streetcode.WebApi/Extensions/ConfigurationBuilderExtensions.cs (AddUserSecrets)- docker-compose.yml (ConnectionStrings__DefaultConnection env var)
Streetcode/Streetcode.BLL/MediatR/Auth/GoogleLogin/GoogleLoginQuery.cs (1)
7-8: LGTM: query shape is appropriate.Streetcode/Streetcode.WebApi/Extensions/ServiceCollectionExtensions.cs (2)
78-79: DI registration is correct.IGoogleService registration + IHttpContextAccessor (Line 175) are necessary for the flow.
175-176: LGTM: IHttpContextAccessor registration.Required for the GoogleService flow.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Streetcode/Streetcode.WebApi/Controllers/Auth/AuthController.cs (1)
1-1: Routing template verified — BaseApiController has [Route("api/[controller]/[action]")]BaseApiController defines [Route("api/[controller]/[action]")], so AuthController's multiple plain [HttpPost] actions are disambiguated by the action segment.
- Note: Streetcode/Streetcode.WebApi/Controllers/FavouriteStreetcode/FavouriteStreetcodeController.cs declares [Route("api/[controller]")] (overrides the base). It currently only has a [HttpDelete], but overriding the base route can cause POST ambiguity if plain [HttpPost] methods are added — consider removing the override or adding explicit action routes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Streetcode/Streetcode.BLL/Streetcode.BLL.csproj(1 hunks)Streetcode/Streetcode.WebApi/Controllers/Auth/AuthController.cs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Streetcode/Streetcode.BLL/Streetcode.BLL.csproj
🧰 Additional context used
🧬 Code graph analysis (1)
Streetcode/Streetcode.WebApi/Controllers/Auth/AuthController.cs (3)
Streetcode/Streetcode.BLL/Interfaces/Google/IGoogleService.cs (1)
Task(6-6)Streetcode/Streetcode.BLL/Services/Google/GoogleService.cs (1)
Task(20-48)Streetcode/Streetcode.BLL/MediatR/Auth/GoogleLogin/GoogleLoginHandler.cs (2)
Task(34-96)Task(98-135)
🪛 GitHub Check: Build and analyze
Streetcode/Streetcode.WebApi/Controllers/Auth/AuthController.cs
[failure] 61-61:
Code should not contain trailing whitespace (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1028.md)
[failure] 61-61:
Code should not contain trailing whitespace (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1028.md)
🪛 GitHub Actions: SonarCloud
Streetcode/Streetcode.WebApi/Controllers/Auth/AuthController.cs
[error] 61-61: SA1028: Code should not contain trailing whitespace.
🔇 Additional comments (2)
Streetcode/Streetcode.WebApi/Controllers/Auth/AuthController.cs (2)
41-47: Mark endpoint [AllowAnonymous], build RedirectUri via Url.Action, and use GoogleDefaults scheme constant.Prevents fallback-policy blocking and avoids hard-coded paths/typos.
- [HttpGet("google")] - public IActionResult GoogleLogin() - { - var props = new AuthenticationProperties { RedirectUri = "/api/auth/google-callback" }; - - return Challenge(props, "Google"); - } + [HttpGet("google")] + [AllowAnonymous] + public IActionResult GoogleLogin() + { + var redirectUri = Url.Action(nameof(GoogleCallback), "Auth", null, Request.Scheme) ?? "/api/auth/google-callback"; + var props = new AuthenticationProperties { RedirectUri = redirectUri }; + + return Challenge(props, GoogleDefaults.AuthenticationScheme); + }Add required usings:
+using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Authentication.Google;
49-61: Callback: add [AllowAnonymous] and clear the external cookie after use.Prevents fallback-policy issues and avoids leaving the temporary external sign-in cookie.
- [HttpGet("google-callback")] - public async Task<IActionResult> GoogleCallback(CancellationToken ct) - { - var result = await Mediator.Send(new GoogleLoginQuery(), ct); - - if (result.IsFailed) - { - return BadRequest(result.Errors.Select(e => e.Message)); - } - - return Ok(result.Value); - } + [HttpGet("google-callback")] + [AllowAnonymous] + public async Task<IActionResult> GoogleCallback(CancellationToken ct) + { + var result = await Mediator.Send(new GoogleLoginQuery(), ct); + + if (result.IsFailed) + { + return BadRequest(result.Errors.Select(e => e.Message)); + } + + await HttpContext.SignOutAsync(IdentityConstants.ExternalScheme); + return Ok(result.Value); + }Add required using:
+using Microsoft.AspNetCore.Identity;
…ttps://github.com/project-studying-dotnet/Streetcode-Server-August-2025 into feature/218/accessibility-logIn-using-google-auth
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Streetcode/Streetcode.WebApi/Extensions/ServiceCollectionExtensions.cs (2)
155-165: Tighten JWT expiration handling.Use zero clock skew to avoid extra grace period on expired tokens.
- IssuerSigningKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(jwtSettings.SecretKey)) + IssuerSigningKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(jwtSettings.SecretKey)), + ClockSkew = TimeSpan.Zero
104-112: Apply configured CORS preflight max age.You define PreflightMaxAge but don’t use it.
.WithMethods(corsConfig.AllowedMethods); + if (corsConfig.PreflightMaxAge > 0) + { + policy.SetPreflightMaxAge(TimeSpan.FromSeconds(corsConfig.PreflightMaxAge)); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Streetcode/Streetcode.WebApi/Extensions/ServiceCollectionExtensions.cs(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and analyze
🔇 Additional comments (6)
Streetcode/Streetcode.WebApi/Extensions/ServiceCollectionExtensions.cs (6)
121-124: Good fix: keep DefaultChallengeScheme = JwtBearer for APIs.Prevents unwanted redirects to Google and correctly returns 401/WWW-Authenticate for API clients.
125-133: Use a dedicated, short‑lived external cookie; drop DefaultSignInScheme.Avoid using the default cookie scheme for the OAuth handshake; prefer a separate scheme with a short TTL and no sliding expiration. This keeps API auth strictly JWT and limits cookie blast radius.
- options.DefaultSignInScheme = CookieAuthenticationDefaults.AuthenticationScheme; - }) - .AddCookie(CookieAuthenticationDefaults.AuthenticationScheme, options => + }) + // Use a short-lived external cookie for the OAuth handshake only + .AddCookie("ExternalAuth", options => { options.Cookie.HttpOnly = true; options.Cookie.SecurePolicy = CookieSecurePolicy.Always; - options.Cookie.SameSite = SameSiteMode.Lax; + options.Cookie.SameSite = SameSiteMode.Lax; // switch to None if cross-site issues appear + options.ExpireTimeSpan = TimeSpan.FromMinutes(5); + options.SlidingExpiration = false; })
133-152: Persist tokens and use the external cookie scheme for Google.Ensure tokens are available if needed downstream and scope includes email.
options.ClientId = clientId; options.ClientSecret = clientSecret; - options.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme; + options.SignInScheme = "ExternalAuth"; + options.SaveTokens = true; + options.Scope.Add("email");
78-79: DI registration for Google service looks good.
175-176: HttpContextAccessor registration is appropriate for Google flow.
168-171: No action required — Google endpoints are already accessible.DefaultPolicy is JwtBearer-only, but AuthController.GoogleLogin and GoogleCallback are not decorated with [Authorize] and explicitly use Challenge("Google") / AuthenticateAsync("Google"), so they remain anonymous. If you later protect these routes, add [AllowAnonymous] here or specify AuthenticationSchemes on the [Authorize] attribute.
|
3 participants
dev
JIRA
Code reviewers
Second Level Review
Summary of issue
ToDo
Summary of change
ToDo
Testing approach
ToDo
CHECK LIST
Summary by CodeRabbit
New Features
Chores
Tests