Skip to content

Commit

Permalink
Feature/quality of life improvements (#21)
Browse files Browse the repository at this point in the history
This bundles up a handful of small changes that makes the application easier to build and run.

* Warning hunt to clean up some existing warnings that have crept in.
* Adds AdminList configuration option, a comma-separated list of emails that are admin accounts
    * This partially implements #4 but the actual admin-y pages need to be added to complete it.
* Adds unit tests for AdminList related functions
* Extraneous forward slashes are dropped from the end of URL lookups, making #6 easier to implement
* Fix the dockerfile to use the right sdk image for building.
* Fix the default okta API urls.
  • Loading branch information
Cellivar authored Jun 1, 2020
1 parent 95c9ce0 commit e8ff427
Show file tree
Hide file tree
Showing 15 changed files with 256 additions and 25 deletions.
2 changes: 1 addition & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ indent_size = 2

# Dotnet diagnostic settings
[*.{cs,vb}]
dotnet_diagnostic.xUnit2018.severity = suppress # "do not compare an object's exact type to the abstract class" is a valid assert, but very noisy right now
# dotnet_diagnostic.xUnit2018.severity = suppress # "do not compare an object's exact type to the abstract class" is a valid assert, but very noisy right now

# Dotnet code style settings:
[*.{cs,vb}]
Expand Down
2 changes: 2 additions & 0 deletions NetGoLynx.Tests/NetGoLynx.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="FakeItEasy" Version="6.0.1" />
<PackageReference Include="FakeItEasy.Analyzer.CSharp" Version="6.0.0" />
<PackageReference Include="Microsoft.AspNetCore.Mvc.Core" Version="2.2.5" />
<PackageReference Include="Microsoft.EntityFrameworkCore.InMemory" Version="3.1.2" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.5.0" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
using NetGoLynx.Models;
using NetGoLynx.Models.Configuration;

namespace NetGoLynx.Tests.UnitTests.Models.Configuration
{
[TestClass]
public class AccountSettingsTest
{
[TestMethod]
public void AdminUsernameReturnsTrue()
{
var goodAccount = new Account()
{
Name = "someaccount@example.com"
};
var badAccount = new Account()
{
Name = "somerando@example.com"
};
var settings = new AccountSettings
{
AdminList = goodAccount.Name
};

Assert.IsTrue(settings.IsAdmin(goodAccount));
Assert.IsFalse(settings.IsAdmin(badAccount));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
using NetGoLynx.Models.RedirectModels;

namespace NetGoLynx.Tests.Models
namespace NetGoLynx.Tests.UnitTests.Models
{
[TestClass]
public class NewRedirectModelTests
Expand All @@ -27,9 +27,9 @@ public void LinkNameIsValid(string name)
{
Target = "https://valid.target"
};
var (IsValid, _) = TryValidate(model);
var (isValid, _) = TryValidate(model);

Assert.IsTrue(IsValid);
Assert.IsTrue(isValid);
}

[DataTestMethod]
Expand All @@ -43,9 +43,9 @@ public void LinkNameIsNotValid(string name)
{
Target = "https://valid.target"
};
var (IsValid, _) = TryValidate(model);
var (isValid, _) = TryValidate(model);

Assert.IsFalse(IsValid);
Assert.IsFalse(isValid);
}

[TestMethod]
Expand Down
112 changes: 112 additions & 0 deletions NetGoLynx.Tests/UnitTests/Models/Services/AccountServiceTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
using System.Collections.Generic;
using System.Security.Claims;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Configuration;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using NetGoLynx.Data;
using NetGoLynx.Models;
using NetGoLynx.Services;

namespace NetGoLynx.Tests.UnitTests.Models.Services
{
[TestClass]
public class AccountServiceTests
{
private static readonly DbContextOptions<RedirectContext> s_dbOps =
new DbContextOptionsBuilder<RedirectContext>()
.UseInMemoryDatabase(databaseName: "account_service_tests")
.Options;

private static int s_accountCount;

private static int AccountCount
{
get
{
s_accountCount++;
return s_accountCount;
}
}

private static Account GetValidAccount()
{
var count = AccountCount;
return new Account()
{
Access = AccessType.Default,
AccountId = count,
Name = $"test_account_{count}@example.com",
};
}

[ClassInitialize]
public static void ClassInitialize(TestContext testContext)
{
testContext.WriteLine("Adding default database entries");
// Add some reference stuff
using var context = new RedirectContext(s_dbOps);
context.Add(GetValidAccount());
context.Add(GetValidAccount());
context.Add(new Account()
{
AccountId = 12345,
Access = AccessType.Default,
Name = "GoodAccount@example.com"
});
context.SaveChanges();
}

[TestMethod]
public void WhitelistedAccountReturnsAsAdminViaAnyMethod()
{
using var context = new RedirectContext(s_dbOps);
var configDict = new Dictionary<string, string>
{
{"AccountSettings:AdminList", "GoodAccount@example.com,OtherGoodAccount@example.com,moregood@example.com"}
};
var config = new ConfigurationBuilder()
.AddInMemoryCollection(configDict)
.Build();
var accountService = new AccountService(context, null, config);

// Standard get methods
Assert.AreEqual(accountService.Get("GoodAccount@example.com").Result.Access, AccessType.Admin);
Assert.AreEqual(accountService.Get(12345).Result.Access, AccessType.Admin);

// ClaimsPrincipal overload
var claims = new ClaimsPrincipal(new ClaimsIdentity(new Claim[]
{
new Claim(ClaimTypes.Email, "GoodAccount@example.com")
}, "mock"));
Assert.AreEqual(accountService.Get(claims).Result.Access, AccessType.Admin);

// Create operation
var newAccount = new Account()
{
Access = AccessType.Default,
Name = "OtherGoodAccount@example.com"
};
var (account, created) = accountService.Create(newAccount).Result;
Assert.IsTrue(created);
Assert.AreEqual(account.Access, AccessType.Admin);

Assert.AreEqual(accountService.GetOrCreate("moregood@example.com").Result.Access, AccessType.Admin);
}

[TestMethod]
public void RegularAccountDoesntReturnAsAdmin()
{
using var context = new RedirectContext(s_dbOps);
var configDict = new Dictionary<string, string>
{
{"AccountSettings:AdminList", "GoodAccount@example.com"}
};
var config = new ConfigurationBuilder()
.AddInMemoryCollection(configDict)
.Build();
var accountService = new AccountService(context, null, config);

Assert.AreNotEqual(accountService.Get("test_account_1@example.com").Result.Access, AccessType.Admin);
}
}
}
8 changes: 4 additions & 4 deletions NetGoLynx/Controllers/AccountController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public AccountController(IAccountService service, IAuthenticationSchemeProvider
/// </summary>
/// <returns>The login selection view.</returns>
[HttpGet("Login")]
public async Task<IActionResult> LoginAsync()
public IActionResult LoginAsync()
{
return View(new LoginModel(_availableSchemes));
}
Expand All @@ -40,7 +40,7 @@ public async Task<IActionResult> LoginAsync()
/// </summary>
/// <returns>A redirect to the list view page.</returns>
[HttpGet("Google")]
public async Task<IActionResult> LoginGoogleAsync()
public IActionResult LoginGoogleAsync()
{
return Challenge(
new AuthenticationProperties
Expand All @@ -55,7 +55,7 @@ public async Task<IActionResult> LoginGoogleAsync()
/// </summary>
/// <returns>A redirect to the list view page.</returns>
[HttpGet("GitHub")]
public async Task<IActionResult> LoginGitHubAsync()
public IActionResult LoginGitHubAsync()
{
return Challenge(
new AuthenticationProperties
Expand All @@ -70,7 +70,7 @@ public async Task<IActionResult> LoginGitHubAsync()
/// </summary>
/// <returns>A redirect to the list view page.</returns>
[HttpGet("Okta")]
public async Task<IActionResult> LoginOktaAsync()
public IActionResult LoginOktaAsync()
{
return Challenge(
new AuthenticationProperties
Expand Down
3 changes: 3 additions & 0 deletions NetGoLynx/Controllers/HomeController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ public IActionResult Index()
[HttpGet("{*name}", Order = int.MaxValue)]
public async Task<IActionResult> ResolveAsync(string name)
{
// Trim extraneous forward slashes only off the end
name = name.TrimEnd('/');

var target = await _redirectService.GetRedirectTargetAsync(name);

if (!string.IsNullOrEmpty(target))
Expand Down
4 changes: 2 additions & 2 deletions NetGoLynx/Controllers/RedirectController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public RedirectController(IRedirectService redirectService)

[HttpGet("notfound")]
[AllowAnonymous]
public IActionResult NotFound()
public new IActionResult NotFound()
{
return View();
}
Expand All @@ -49,7 +49,7 @@ public async Task<IActionResult> ListAsync(int highlightId)
/// <param name="suggestedLinkName">An optional suggested link name to add.</param>
/// <returns>The add view</returns>
[HttpGet("add")]
public async Task<IActionResult> AddAsync(string suggestedLinkName = "")
public IActionResult Add(string suggestedLinkName = "")
{
return View("Add", new RedirectMetadata(suggestedLinkName));
}
Expand Down
2 changes: 1 addition & 1 deletion NetGoLynx/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ WORKDIR /app
EXPOSE 80
EXPOSE 443

FROM mcr.microsoft.com/dotnet/core/aspnet:3.1-alpine AS build
FROM mcr.microsoft.com/dotnet/core/sdk:3.1-alpine AS build
WORKDIR /src
COPY ["NetGoLynx/NetGoLynx.csproj", "NetGoLynx/"]
RUN dotnet restore "NetGoLynx/NetGoLynx.csproj"
Expand Down
6 changes: 6 additions & 0 deletions NetGoLynx/Models/AccessType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,14 @@ namespace NetGoLynx.Models
[Flags]
public enum AccessType
{
/// <summary>
/// Standard user access level
/// </summary>
Default = 0,

/// <summary>
/// Complete control access level
/// </summary>
Admin = 1,
}
}
37 changes: 37 additions & 0 deletions NetGoLynx/Models/Configuration/AccountSettings.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
using System.Collections.Generic;

namespace NetGoLynx.Models.Configuration
{
/// <summary>
/// Model for Account-related settings.
/// </summary>
public class AccountSettings
{
private HashSet<string> _adminList;

private string _whitelist;

/// <summary>
/// Gets or sets the comma-separated list of accounts that have admin.
/// </summary>
public string AdminList
{
get => _whitelist;
set
{
_whitelist = value;
_adminList = new HashSet<string>(_whitelist.Split(','));
}
}

/// <summary>
/// Determine if an account is an admin.
/// </summary>
/// <param name="account">The account to check</param>
/// <returns>True if the username exactly matches an entry in the whitelist.</returns>
public bool IsAdmin(Account account)
{
return account != null && _adminList.Contains(account.Name);
}
}
}
18 changes: 15 additions & 3 deletions NetGoLynx/Models/Configuration/Authentication/Okta.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,29 @@ public class Okta
/// <summary>
/// Gets or sets the OAuth auth endpoint URL
/// </summary>
public string AuthorizationEndpoint => $"{OktaDomain}/oauth2/default/v1/authorize";
/// <remarks>
/// No /default/ between oauth2 and v1 here because that only applies to
/// custom auth servers: https://developer.okta.com/docs/reference/api/oidc/
/// </remarks>
public string AuthorizationEndpoint => $"{OktaDomain}/oauth2/v1/authorize";

/// <summary>
/// Gets or sets the OAuth token endpoint URL
/// </summary>
public string TokenEndpoint => $"{OktaDomain}/oauth2/default/v1/token";
/// <remarks>
/// No /default/ between oauth2 and v1 here because that only applies to
/// custom auth servers: https://developer.okta.com/docs/reference/api/oidc/
/// </remarks>
public string TokenEndpoint => $"{OktaDomain}/oauth2/v1/token";

/// <summary>
/// Gets or sets the user information URL
/// </summary>
public string UserInformationEndpoint => $"{OktaDomain}/oauth2/default/v1/userinfo";
/// <remarks>
/// No /default/ between oauth2 and v1 here because that only applies to
/// custom auth servers: https://developer.okta.com/docs/reference/api/oidc/
/// </remarks>
public string UserInformationEndpoint => $"{OktaDomain}/oauth2/v1/userinfo";

/// <summary>
/// Gets or sets the okta domain.
Expand Down
Loading

0 comments on commit e8ff427

Please sign in to comment.