Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/security issues #261

Merged
merged 6 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## [Unreleased]

### Fixed

- Security vulnerabilities

## [3.14.4] - 2023-11-17

### Fixes
### Fixed

- Locales substring start index

Expand Down
51 changes: 34 additions & 17 deletions dotnet/GraphQL/Mutation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,33 @@ public Mutation(IProductReviewService productReviewService)
{
Name = "Mutation";

Field<ReviewType>(
FieldAsync<ReviewType>(
"newReview",
arguments: new QueryArguments(
new QueryArgument<NonNullGraphType<ReviewInputType>> {Name = "review"}
),
resolve: context =>
resolve: async context =>
{
AppSettings appSettings = await productReviewService.GetAppSettings();
if (!appSettings.AllowAnonymousReviews)
{
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>("review");
return productReviewService.NewReview(review, true);
});
return await productReviewService.NewReview(review, true);
}
);

FieldAsync<ReviewType>(
"editReview",
Expand All @@ -33,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;
Expand All @@ -58,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;
Expand All @@ -82,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;
Expand All @@ -102,4 +119,4 @@ public Mutation(IProductReviewService productReviewService)
);
}
}
}
}
31 changes: 30 additions & 1 deletion dotnet/GraphQL/Query.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -47,6 +48,34 @@ public Query(IProductReviewService productReviewService)
int to = context.GetArgument<int>("to") + 1;
string orderBy = context.GetArgument<string>("orderBy");
string status = context.GetArgument<string>("status");

if (string.IsNullOrEmpty(status) || (!string.IsNullOrEmpty(status) && status.Equals("false")))
{
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<Review>() },
Range = new SearchRange
{
Total = 0,
From = 0,
To = 0
}
};
}

}
}

var searchResult = await productReviewService.GetReviews(searchTerm, from, to, orderBy, status);
SearchResponse searchResponse = new SearchResponse
{
Expand Down Expand Up @@ -269,4 +298,4 @@ public Query(IProductReviewService productReviewService)
);
}
}
}
}
3 changes: 3 additions & 0 deletions dotnet/Models/ValidatedUser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
}
}
5 changes: 3 additions & 2 deletions dotnet/ReviewsRatings.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="GraphQL" Version="2.4.0" />
<PackageReference Include="Vtex.Api" Version="0.1.0-beta" />
<FrameworkReference Include="Microsoft.AspNetCore.App" />
<PackageReference Include="GraphQL" Version="2.4.0" />
<PackageReference Include="Newtonsoft.Json" Version="12.0.3" />
<PackageReference Include="Vtex.Api" Version="0.1.1" />
</ItemGroup>

</Project>
2 changes: 2 additions & 0 deletions dotnet/Services/IProductReviewService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ public interface IProductReviewService

Task<HttpStatusCode> IsValidAuthUser();

Task<HttpStatusCode> IsAdminAuthUser();

Task<ReviewsResponseWrapper> GetReviewsByShopperId(string shopperId);

Task<ReviewsResponseWrapper> GetReviewsByreviewDateTime(string reviewDateTime);
Expand Down
53 changes: 49 additions & 4 deletions dotnet/Services/ProductReviewService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using ReviewsRatings.DataSources;
using Vtex.Api.Context;
using ReviewsRatings.Utils;
using Microsoft.AspNetCore.Http;

/// <summary>
/// Business logic
Expand All @@ -20,16 +21,21 @@
{
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));
}
Expand Down Expand Up @@ -236,7 +242,7 @@
return reviews;
}

public async Task<IList<Review>> LimitReviews(IList<Review> reviews, int from, int to)

Check warning on line 245 in dotnet/Services/ProductReviewService.cs

View workflow job for this annotation

GitHub Actions / QE / Lint .Net

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
{
int take = maximumReturnedRecords;
if (to > 0)
Expand All @@ -249,7 +255,7 @@
return reviews;
}

public async Task<IList<Review>> FilterReviews(IList<Review> reviews, string searchTerm, string orderBy, string status)

Check warning on line 258 in dotnet/Services/ProductReviewService.cs

View workflow job for this annotation

GitHub Actions / QE / Lint .Net

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
{
if (reviews != null && reviews.Count > 0)
{
Expand Down Expand Up @@ -649,14 +655,21 @@

public async Task<HttpStatusCode> IsValidAuthUser()
{
if (string.IsNullOrEmpty(_context.Vtex.AdminUserAuthToken))
string VtexIdclientAutCookieKey = this._httpContextAccessor.HttpContext.Request.Headers["VtexIdclientAutCookie"];

if (string.IsNullOrEmpty(_context.Vtex.StoreUserAuthToken) && 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)
{
Expand All @@ -665,8 +678,10 @@
}

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;
Expand All @@ -675,6 +690,36 @@
return HttpStatusCode.OK;
}

public async Task<HttpStatusCode> 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") && validatedAdminUser.Account.Equals(_context.Vtex.Account) && validatedAdminUser.Audience.Equals("admin");

if (!hasAdminPermission)
{
_context.Vtex.Logger.Warn("IsAdminAuthUser", null, "User Does Not Have Permission");
return HttpStatusCode.Forbidden;
}

return HttpStatusCode.OK;
}

public async Task<bool> ValidateKeyAndToken(string key, string token, string baseUrl)
{
return await this._productReviewRepository.ValidateKeyAndToken(key, token, baseUrl);
Expand Down Expand Up @@ -721,7 +766,7 @@
return await _productReviewRepository.SuccessfulMigration();
}

private async Task<Review> ConvertLegacyReview(LegacyReview review)

Check warning on line 769 in dotnet/Services/ProductReviewService.cs

View workflow job for this annotation

GitHub Actions / QE / Lint .Net

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
{
Review newReview = new Review
{
Expand Down Expand Up @@ -865,7 +910,7 @@
return retval;
}

private async Task<string> GetSortQuery(string orderBy)

Check warning on line 913 in dotnet/Services/ProductReviewService.cs

View workflow job for this annotation

GitHub Actions / QE / Lint .Net

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
{
string sort = string.Empty;
if (!string.IsNullOrEmpty(orderBy))
Expand Down
Loading