From 9d903a2dbc69b24448b7f9427bdf0c450922f8c5 Mon Sep 17 00:00:00 2001 From: Edwin Obando Date: Thu, 9 May 2024 17:35:07 -0500 Subject: [PATCH 1/6] fix: :lock: auth on private mutations ands queries --- dotnet/GraphQL/Mutation.cs | 23 +++++++--- dotnet/GraphQL/Query.cs | 11 ++++- dotnet/ReviewsRatings.csproj | 5 ++- dotnet/Services/IProductReviewService.cs | 2 + dotnet/Services/ProductReviewService.cs | 53 ++++++++++++++++++++++-- 5 files changed, 82 insertions(+), 12 deletions(-) diff --git a/dotnet/GraphQL/Mutation.cs b/dotnet/GraphQL/Mutation.cs index 9c42ccab..1570b724 100644 --- a/dotnet/GraphQL/Mutation.cs +++ b/dotnet/GraphQL/Mutation.cs @@ -14,16 +14,29 @@ public Mutation(IProductReviewService productReviewService) { Name = "Mutation"; - Field( + FieldAsync( "newReview", arguments: new QueryArguments( new QueryArgument> {Name = "review"} ), - resolve: context => + resolve: async context => { + HttpStatusCode isValidAuthUser = await productReviewService.IsValidAuthUser(); + + if (isValidAuthUser != HttpStatusCode.OK) + { + context.Errors.Add(new ExecutionError(isValidAuthUser.ToString()) + { + Code = isValidAuthUser.ToString() + }); + + return default; + } + var review = context.GetArgument("review"); - return productReviewService.NewReview(review, true); - }); + return await productReviewService.NewReview(review, true); + } + ); FieldAsync( "editReview", @@ -102,4 +115,4 @@ public Mutation(IProductReviewService productReviewService) ); } } -} \ No newline at end of file +} diff --git a/dotnet/GraphQL/Query.cs b/dotnet/GraphQL/Query.cs index 655a0273..c323e15e 100644 --- a/dotnet/GraphQL/Query.cs +++ b/dotnet/GraphQL/Query.cs @@ -4,6 +4,7 @@ using ReviewsRatings.Models; using ReviewsRatings.Services; using System; +using System.Net; using System.Collections; using System.Collections.Generic; using System.Linq; @@ -47,6 +48,14 @@ public Query(IProductReviewService productReviewService) int to = context.GetArgument("to") + 1; string orderBy = context.GetArgument("orderBy"); string status = context.GetArgument("status"); + + HttpStatusCode isAdminAuthUser = await productReviewService.IsAdminAuthUser(); + + if (isAdminAuthUser != HttpStatusCode.OK) + { + status = "true"; + } + var searchResult = await productReviewService.GetReviews(searchTerm, from, to, orderBy, status); SearchResponse searchResponse = new SearchResponse { @@ -269,4 +278,4 @@ public Query(IProductReviewService productReviewService) ); } } -} \ No newline at end of file +} diff --git a/dotnet/ReviewsRatings.csproj b/dotnet/ReviewsRatings.csproj index af4e1455..7c89dbc4 100644 --- a/dotnet/ReviewsRatings.csproj +++ b/dotnet/ReviewsRatings.csproj @@ -5,9 +5,10 @@ - - + + + diff --git a/dotnet/Services/IProductReviewService.cs b/dotnet/Services/IProductReviewService.cs index 4efe47cc..00470f2a 100644 --- a/dotnet/Services/IProductReviewService.cs +++ b/dotnet/Services/IProductReviewService.cs @@ -28,6 +28,8 @@ public interface IProductReviewService Task IsValidAuthUser(); + Task IsAdminAuthUser(); + Task GetReviewsByShopperId(string shopperId); Task GetReviewsByreviewDateTime(string reviewDateTime); diff --git a/dotnet/Services/ProductReviewService.cs b/dotnet/Services/ProductReviewService.cs index 51fb464e..4e8f1d70 100644 --- a/dotnet/Services/ProductReviewService.cs +++ b/dotnet/Services/ProductReviewService.cs @@ -12,6 +12,7 @@ using ReviewsRatings.DataSources; using Vtex.Api.Context; using ReviewsRatings.Utils; + using Microsoft.AspNetCore.Http; /// /// Business logic @@ -20,16 +21,21 @@ public class ProductReviewService : IProductReviewService { private readonly IProductReviewRepository _productReviewRepository; private readonly IAppSettingsRepository _appSettingsRepository; + private readonly IHttpContextAccessor _httpContextAccessor; private readonly IIOServiceContext _context; + private const int maximumReturnedRecords = 500; private const string DELIMITER = ":"; - public ProductReviewService(IProductReviewRepository productReviewRepository, IAppSettingsRepository appSettingsRepository, IIOServiceContext context) + public ProductReviewService(IProductReviewRepository productReviewRepository, IAppSettingsRepository appSettingsRepository, IHttpContextAccessor httpContextAccessor, IIOServiceContext context) { this._productReviewRepository = productReviewRepository ?? throw new ArgumentNullException(nameof(productReviewRepository)); this._appSettingsRepository = appSettingsRepository ?? throw new ArgumentNullException(nameof(appSettingsRepository)); + + this._httpContextAccessor = httpContextAccessor ?? + throw new ArgumentNullException(nameof(httpContextAccessor)); this._context = context ?? throw new ArgumentNullException(nameof(context)); } @@ -649,14 +655,21 @@ public async Task ValidateUserToken(string token) public async Task IsValidAuthUser() { - if (string.IsNullOrEmpty(_context.Vtex.AdminUserAuthToken)) + string VtexIdclientAutCookieKey = this._httpContextAccessor.HttpContext.Request.Headers["VtexIdclientAutCookie"]; + + if (string.IsNullOrEmpty(_context.Vtex.AdminUserAuthToken) && string.IsNullOrEmpty(VtexIdclientAutCookieKey)) { return HttpStatusCode.Unauthorized; } ValidatedUser validatedUser = null; + ValidatedUser validatedAdminUser = null; + ValidatedUser validatedKeyApp = null; + try { - validatedUser = await ValidateUserToken(_context.Vtex.AdminUserAuthToken); + validatedUser = await ValidateUserToken(_context.Vtex.StoreUserAuthToken); + validatedAdminUser = await ValidateUserToken(_context.Vtex.AdminUserAuthToken); + validatedKeyApp = await ValidateUserToken(VtexIdclientAutCookieKey); } catch (Exception ex) { @@ -665,8 +678,10 @@ public async Task IsValidAuthUser() } bool hasPermission = validatedUser != null && validatedUser.AuthStatus.Equals("Success"); + bool hasAdminPermission = validatedAdminUser != null && validatedAdminUser.AuthStatus.Equals("Success"); + bool hasPermissionToken = validatedKeyApp != null && validatedKeyApp.AuthStatus.Equals("Success"); - if (!hasPermission) + if (!hasPermission && !hasAdminPermission && !hasPermissionToken) { _context.Vtex.Logger.Warn("IsValidAuthUser", null, "User Does Not Have Permission"); return HttpStatusCode.Forbidden; @@ -675,6 +690,36 @@ public async Task IsValidAuthUser() return HttpStatusCode.OK; } + public async Task IsAdminAuthUser() + { + + if (string.IsNullOrEmpty(_context.Vtex.AdminUserAuthToken)) + { + return HttpStatusCode.Unauthorized; + } + + ValidatedUser validatedAdminUser = null; + + try { + validatedAdminUser = await ValidateUserToken(_context.Vtex.AdminUserAuthToken); + } + catch (Exception ex) + { + _context.Vtex.Logger.Error("IsAdminAuthUser", null, "Error fetching user", ex); + return HttpStatusCode.BadRequest; + } + + bool hasAdminPermission = validatedAdminUser != null && validatedAdminUser.AuthStatus.Equals("Success"); + + if (!hasAdminPermission) + { + _context.Vtex.Logger.Warn("IsAdminAuthUser", null, "User Does Not Have Permission"); + return HttpStatusCode.Forbidden; + } + + return HttpStatusCode.OK; + } + public async Task ValidateKeyAndToken(string key, string token, string baseUrl) { return await this._productReviewRepository.ValidateKeyAndToken(key, token, baseUrl); From 8b5bb2b1e8171ade1ed989615c36ab32a9b67c3a Mon Sep 17 00:00:00 2001 From: Edwin Obando Date: Thu, 9 May 2024 17:36:09 -0500 Subject: [PATCH 2/6] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad7765ca..e0f4776b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Fixes + +- Security vulnerabilities + ## [3.14.4] - 2023-11-17 ### Fixes From f40ce9cf10f820c44b56d3f10c2280f67115bbf4 Mon Sep 17 00:00:00 2001 From: Edwin Obando Date: Fri, 10 May 2024 12:52:48 -0500 Subject: [PATCH 3/6] Update CHANGELOG.md --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0f4776b..bf7a1632 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,13 +6,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] -### Fixes +### Fixed - Security vulnerabilities ## [3.14.4] - 2023-11-17 -### Fixes +### Fixed - Locales substring start index From 5b0f26e27239b65f3354fd80ea644047f7cb6a49 Mon Sep 17 00:00:00 2001 From: Edwin Obando Date: Thu, 16 May 2024 13:30:32 -0500 Subject: [PATCH 4/6] fix: include StoreUserAuthToken in validation --- dotnet/Services/ProductReviewService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/Services/ProductReviewService.cs b/dotnet/Services/ProductReviewService.cs index 4e8f1d70..baf1c4a1 100644 --- a/dotnet/Services/ProductReviewService.cs +++ b/dotnet/Services/ProductReviewService.cs @@ -657,7 +657,7 @@ public async Task IsValidAuthUser() { string VtexIdclientAutCookieKey = this._httpContextAccessor.HttpContext.Request.Headers["VtexIdclientAutCookie"]; - if (string.IsNullOrEmpty(_context.Vtex.AdminUserAuthToken) && string.IsNullOrEmpty(VtexIdclientAutCookieKey)) + if (string.IsNullOrEmpty(_context.Vtex.StoreUserAuthToken) && string.IsNullOrEmpty(_context.Vtex.AdminUserAuthToken) && string.IsNullOrEmpty(VtexIdclientAutCookieKey)) { return HttpStatusCode.Unauthorized; } From dd67e06a4a8a272ad049163aa2868c0586c377ef Mon Sep 17 00:00:00 2001 From: Edwin Obando Date: Thu, 16 May 2024 15:42:18 -0500 Subject: [PATCH 5/6] fix: support Anonymous Reviews --- dotnet/GraphQL/Mutation.cs | 44 +++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/dotnet/GraphQL/Mutation.cs b/dotnet/GraphQL/Mutation.cs index 1570b724..e6a799dc 100644 --- a/dotnet/GraphQL/Mutation.cs +++ b/dotnet/GraphQL/Mutation.cs @@ -21,16 +21,20 @@ public Mutation(IProductReviewService productReviewService) ), resolve: async context => { - HttpStatusCode isValidAuthUser = await productReviewService.IsValidAuthUser(); - - if (isValidAuthUser != HttpStatusCode.OK) + AppSettings appSettings = await productReviewService.GetAppSettings(); + if (!appSettings.AllowAnonymousReviews) { - context.Errors.Add(new ExecutionError(isValidAuthUser.ToString()) + HttpStatusCode isValidAuthUser = await productReviewService.IsValidAuthUser(); + + if (isValidAuthUser != HttpStatusCode.OK) { - Code = isValidAuthUser.ToString() - }); - - return default; + context.Errors.Add(new ExecutionError(isValidAuthUser.ToString()) + { + Code = isValidAuthUser.ToString() + }); + + return default; + } } var review = context.GetArgument("review"); @@ -46,13 +50,13 @@ public Mutation(IProductReviewService productReviewService) ), resolve: async context => { - HttpStatusCode isValidAuthUser = await productReviewService.IsValidAuthUser(); + HttpStatusCode IsAdminAuthUser = await productReviewService.IsAdminAuthUser(); - if (isValidAuthUser != HttpStatusCode.OK) + if (IsAdminAuthUser != HttpStatusCode.OK) { - context.Errors.Add(new ExecutionError(isValidAuthUser.ToString()) + context.Errors.Add(new ExecutionError(IsAdminAuthUser.ToString()) { - Code = isValidAuthUser.ToString() + Code = IsAdminAuthUser.ToString() }); return default; @@ -71,13 +75,13 @@ public Mutation(IProductReviewService productReviewService) ), resolve: async context => { - HttpStatusCode isValidAuthUser = await productReviewService.IsValidAuthUser(); + HttpStatusCode IsAdminAuthUser = await productReviewService.IsAdminAuthUser(); - if (isValidAuthUser != HttpStatusCode.OK) + if (IsAdminAuthUser != HttpStatusCode.OK) { - context.Errors.Add(new ExecutionError(isValidAuthUser.ToString()) + context.Errors.Add(new ExecutionError(IsAdminAuthUser.ToString()) { - Code = isValidAuthUser.ToString() + Code = IsAdminAuthUser.ToString() }); return default; @@ -95,13 +99,13 @@ public Mutation(IProductReviewService productReviewService) ), resolve: async context => { - HttpStatusCode isValidAuthUser = await productReviewService.IsValidAuthUser(); + HttpStatusCode IsAdminAuthUser = await productReviewService.IsAdminAuthUser(); - if (isValidAuthUser != HttpStatusCode.OK) + if (IsAdminAuthUser != HttpStatusCode.OK) { - context.Errors.Add(new ExecutionError(isValidAuthUser.ToString()) + context.Errors.Add(new ExecutionError(IsAdminAuthUser.ToString()) { - Code = isValidAuthUser.ToString() + Code = IsAdminAuthUser.ToString() }); return default; From ddd199ac5c02cdc24bfb8ae851376291dd23919e Mon Sep 17 00:00:00 2001 From: Edwin Obando Date: Tue, 21 May 2024 20:14:19 -0500 Subject: [PATCH 6/6] fix: only admin users, cross-account token and empty response --- dotnet/GraphQL/Query.cs | 28 +++++++++++++++++++++---- dotnet/Models/ValidatedUser.cs | 3 +++ dotnet/Services/ProductReviewService.cs | 2 +- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/dotnet/GraphQL/Query.cs b/dotnet/GraphQL/Query.cs index c323e15e..337172be 100644 --- a/dotnet/GraphQL/Query.cs +++ b/dotnet/GraphQL/Query.cs @@ -49,11 +49,31 @@ public Query(IProductReviewService productReviewService) string orderBy = context.GetArgument("orderBy"); string status = context.GetArgument("status"); - HttpStatusCode isAdminAuthUser = await productReviewService.IsAdminAuthUser(); - - if (isAdminAuthUser != HttpStatusCode.OK) + if (string.IsNullOrEmpty(status) || (!string.IsNullOrEmpty(status) && status.Equals("false"))) { - status = "true"; + HttpStatusCode isAdminAuthUser = await productReviewService.IsAdminAuthUser(); + + if (isAdminAuthUser != HttpStatusCode.OK) + { + if (string.IsNullOrEmpty(status)) + { + status = "true"; + } + else + { + return new SearchResponse + { + Data = new DataElement { data = new List() }, + Range = new SearchRange + { + Total = 0, + From = 0, + To = 0 + } + }; + } + + } } var searchResult = await productReviewService.GetReviews(searchTerm, from, to, orderBy, status); diff --git a/dotnet/Models/ValidatedUser.cs b/dotnet/Models/ValidatedUser.cs index ca61fd83..4aee43c2 100644 --- a/dotnet/Models/ValidatedUser.cs +++ b/dotnet/Models/ValidatedUser.cs @@ -9,5 +9,8 @@ public class ValidatedUser public string AuthStatus { get; set; } public string Id { get; set; } public string User { get; set; } // email + public string Account { get; set; } + public string Audience { get; set; } + public string TokenType { get; set; } } } diff --git a/dotnet/Services/ProductReviewService.cs b/dotnet/Services/ProductReviewService.cs index baf1c4a1..4a39ce59 100644 --- a/dotnet/Services/ProductReviewService.cs +++ b/dotnet/Services/ProductReviewService.cs @@ -709,7 +709,7 @@ public async Task IsAdminAuthUser() return HttpStatusCode.BadRequest; } - bool hasAdminPermission = validatedAdminUser != null && validatedAdminUser.AuthStatus.Equals("Success"); + bool hasAdminPermission = validatedAdminUser != null && validatedAdminUser.AuthStatus.Equals("Success") && validatedAdminUser.Account.Equals(_context.Vtex.Account) && validatedAdminUser.Audience.Equals("admin"); if (!hasAdminPermission) {