Skip to content

Commit 1f0fd38

Browse files
niemyjskiniemyjski
committed
Validate external account state and identity
1 parent b6c99e2 commit 1f0fd38

6 files changed

Lines changed: 208 additions & 3 deletions

File tree

‎src/Exceptionless.Core/Repositories/UserRepository.cs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,19 @@ public Task<bool> SetSavedViewOrdersAsync(User user, CommandOptionsDescriptor<Us
129129
return null;
130130

131131
provider = provider.ToLowerInvariant();
132-
var results = (await FindAsync(q => q.FieldEquals(u => u.OAuthAccounts.First().ProviderUserId, providerUserId))).Documents;
133-
return results.FirstOrDefault(u => u.OAuthAccounts.Any(o => o.Provider == provider));
132+
var results = await FindAsync(q => q.FieldEquals(u => u.OAuthAccounts.First().ProviderUserId, providerUserId),
133+
o => o.SearchAfterPaging().PageLimit(100));
134+
User? owner = null;
135+
do
136+
{
137+
foreach (var user in results.Documents.Where(u => u.OAuthAccounts.Any(o => o.Provider == provider && o.ProviderUserId == providerUserId)))
138+
{
139+
if (owner is not null)
140+
throw new UnauthorizedAccessException("The external account is linked to multiple users.");
141+
owner = user;
142+
}
143+
} while (await results.NextPageAsync());
144+
return owner;
134145
}
135146

136147
public async Task<User?> GetByVerifyEmailAddressTokenAsync(string token)

‎src/Exceptionless.Web/Api/Endpoints/AuthEndpoints.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,14 @@ headers api_key input box.
104104
.AllowAnonymous()
105105
.Accepts<ExternalAuthInfo>("application/json", "application/*+json")
106106
.Produces<TokenResult>()
107+
.ProducesProblem(StatusCodes.Status401Unauthorized)
107108
.ProducesProblem(StatusCodes.Status403Forbidden)
108109
.ProducesProblem(StatusCodes.Status422UnprocessableEntity)
109110
.WithSummary("Sign in with GitHub")
110111
.WithMetadata(new EndpointDocumentation {
111112
ResponseDescriptions = new() {
112113
["200"] = "User Authentication Token",
114+
["401"] = "Sign-in failed",
113115
["403"] = "Account Creation is currently disabled",
114116
["422"] = "Validation error",
115117
}
@@ -122,12 +124,14 @@ headers api_key input box.
122124
.AllowAnonymous()
123125
.Accepts<ExternalAuthInfo>("application/json", "application/*+json")
124126
.Produces<TokenResult>()
127+
.ProducesProblem(StatusCodes.Status401Unauthorized)
125128
.ProducesProblem(StatusCodes.Status403Forbidden)
126129
.ProducesProblem(StatusCodes.Status422UnprocessableEntity)
127130
.WithSummary("Sign in with Google")
128131
.WithMetadata(new EndpointDocumentation {
129132
ResponseDescriptions = new() {
130133
["200"] = "User Authentication Token",
134+
["401"] = "Sign-in failed",
131135
["403"] = "Account Creation is currently disabled",
132136
["422"] = "Validation error",
133137
}
@@ -140,12 +144,14 @@ headers api_key input box.
140144
.AllowAnonymous()
141145
.Accepts<ExternalAuthInfo>("application/json", "application/*+json")
142146
.Produces<TokenResult>()
147+
.ProducesProblem(StatusCodes.Status401Unauthorized)
143148
.ProducesProblem(StatusCodes.Status403Forbidden)
144149
.ProducesProblem(StatusCodes.Status422UnprocessableEntity)
145150
.WithSummary("Sign in with Facebook")
146151
.WithMetadata(new EndpointDocumentation {
147152
ResponseDescriptions = new() {
148153
["200"] = "User Authentication Token",
154+
["401"] = "Sign-in failed",
149155
["403"] = "Account Creation is currently disabled",
150156
["422"] = "Validation error",
151157
}
@@ -158,12 +164,14 @@ headers api_key input box.
158164
.AllowAnonymous()
159165
.Accepts<ExternalAuthInfo>("application/json", "application/*+json")
160166
.Produces<TokenResult>()
167+
.ProducesProblem(StatusCodes.Status401Unauthorized)
161168
.ProducesProblem(StatusCodes.Status403Forbidden)
162169
.ProducesProblem(StatusCodes.Status422UnprocessableEntity)
163170
.WithSummary("Sign in with Microsoft")
164171
.WithMetadata(new EndpointDocumentation {
165172
ResponseDescriptions = new() {
166173
["200"] = "User Authentication Token",
174+
["401"] = "Sign-in failed",
167175
["403"] = "Account Creation is currently disabled",
168176
["422"] = "Validation error",
169177
}

‎src/Exceptionless.Web/Api/Handlers/AuthHandler.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,11 @@ private async Task<Result<TokenResult>> ExternalLoginAsync(ExternalAuthInfo auth
524524
{
525525
user = await FromExternalLoginAsync(userInfo, authInfo.InviteToken, httpContext);
526526
}
527+
catch (UnauthorizedAccessException ex)
528+
{
529+
logger.LogWarning(ex, "External login failed for {EmailAddress}: Account unavailable", userInfo.Email);
530+
return Result.Unauthorized("Login failed.");
531+
}
527532
catch (ApplicationException ex)
528533
{
529534
logger.LogCritical(ex, "External login failed for {EmailAddress}: {Message}", userInfo.Email, ex.Message);
@@ -551,9 +556,13 @@ private async Task<User> FromExternalLoginAsync(UserInfo userInfo, string? invit
551556
var existingUser = await userRepository.GetUserByOAuthProviderAsync(userInfo.ProviderName, userInfo.Id);
552557
using var _ = logger.BeginScope(new ExceptionlessState().Tag("External Login").Tag(userInfo.ProviderName).Identity(userInfo.Email).SetHttpContext(httpContext));
553558

559+
if (existingUser is not null)
560+
EnsureUserIsActive(existingUser);
561+
554562
if (String.IsNullOrWhiteSpace(inviteToken) && httpContext.User.IsUserAuthType())
555563
{
556564
var currentUser = httpContext.Request.GetUser();
565+
EnsureUserIsActive(currentUser);
557566
if (existingUser is not null)
558567
{
559568
if (!String.Equals(existingUser.Id, currentUser.Id, StringComparison.Ordinal))
@@ -598,6 +607,7 @@ private async Task<User> FromExternalLoginAsync(UserInfo userInfo, string? invit
598607
await AddGlobalAdminRoleIfFirstUserAsync(user);
599608
}
600609

610+
EnsureUserIsActive(user);
601611
user.MarkEmailAddressVerified();
602612
user.AddOAuthAccount(userInfo.ProviderName, userInfo.Id, userInfo.Email);
603613

@@ -609,6 +619,12 @@ private async Task<User> FromExternalLoginAsync(UserInfo userInfo, string? invit
609619
return user;
610620
}
611621

622+
private static void EnsureUserIsActive(User user)
623+
{
624+
if (!user.IsActive)
625+
throw new UnauthorizedAccessException("The user is inactive.");
626+
}
627+
612628
private async Task<bool> IsAccountCreationEnabledAsync(string? token)
613629
{
614630
if (authOptions.EnableAccountCreation)

‎tests/Exceptionless.Tests/Api/Data/openapi.json‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1731,6 +1731,16 @@
17311731
}
17321732
}
17331733
},
1734+
"401": {
1735+
"description": "Sign-in failed",
1736+
"content": {
1737+
"application/problem\u002Bjson": {
1738+
"schema": {
1739+
"$ref": "#/components/schemas/ProblemDetails"
1740+
}
1741+
}
1742+
}
1743+
},
17341744
"403": {
17351745
"description": "Account Creation is currently disabled",
17361746
"content": {
@@ -1786,6 +1796,16 @@
17861796
}
17871797
}
17881798
},
1799+
"401": {
1800+
"description": "Sign-in failed",
1801+
"content": {
1802+
"application/problem\u002Bjson": {
1803+
"schema": {
1804+
"$ref": "#/components/schemas/ProblemDetails"
1805+
}
1806+
}
1807+
}
1808+
},
17891809
"403": {
17901810
"description": "Account Creation is currently disabled",
17911811
"content": {
@@ -1841,6 +1861,16 @@
18411861
}
18421862
}
18431863
},
1864+
"401": {
1865+
"description": "Sign-in failed",
1866+
"content": {
1867+
"application/problem\u002Bjson": {
1868+
"schema": {
1869+
"$ref": "#/components/schemas/ProblemDetails"
1870+
}
1871+
}
1872+
}
1873+
},
18441874
"403": {
18451875
"description": "Account Creation is currently disabled",
18461876
"content": {
@@ -1896,6 +1926,16 @@
18961926
}
18971927
}
18981928
},
1929+
"401": {
1930+
"description": "Sign-in failed",
1931+
"content": {
1932+
"application/problem\u002Bjson": {
1933+
"schema": {
1934+
"$ref": "#/components/schemas/ProblemDetails"
1935+
}
1936+
}
1937+
}
1938+
},
18991939
"403": {
19001940
"description": "Account Creation is currently disabled",
19011941
"content": {
@@ -15200,4 +15240,4 @@
1520015240
"name": "Source Map"
1520115241
}
1520215242
]
15203-
}
15243+
}

‎tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1995,6 +1995,134 @@ await SendRequestAsync(r => r
19951995
Assert.False(token.IsSuspended);
19961996
}
19971997

1998+
[Theory]
1999+
[InlineData(false)]
2000+
[InlineData(true)]
2001+
public async Task ExternalLogin_InactiveUser_IsRejectedWithoutChangingAccountOrInvite(bool hasExistingLink)
2002+
{
2003+
// Arrange
2004+
const string code = "github-inactive-user";
2005+
string email = TestOAuthProviderClient.GetEmailAddress(code);
2006+
var user = new User
2007+
{
2008+
EmailAddress = email,
2009+
FullName = "Inactive User",
2010+
IsActive = false,
2011+
Roles = new HashSet<string>([AuthorizationRoles.Client, AuthorizationRoles.User])
2012+
};
2013+
user.ResetVerifyEmailAddressTokenAndExpiration(TimeProvider);
2014+
if (hasExistingLink)
2015+
user.AddOAuthAccount("github", code, email);
2016+
await _userRepository.AddAsync(user, options => options.ImmediateConsistency());
2017+
var organization = (await _organizationRepository.GetAllAsync()).Documents.First();
2018+
var invite = new Invite
2019+
{
2020+
Token = StringExtensions.GetNewToken(),
2021+
EmailAddress = email,
2022+
DateAdded = TimeProvider.GetUtcNow().UtcDateTime
2023+
};
2024+
organization.Invites.Add(invite);
2025+
await _organizationRepository.SaveAsync(organization, options => options.ImmediateConsistency());
2026+
2027+
// Act
2028+
await SendRequestAsync(request => request
2029+
.Post()
2030+
.AppendPaths("auth", "github")
2031+
.Content(new ExternalAuthInfo
2032+
{
2033+
ClientId = "client-id",
2034+
Code = code,
2035+
InviteToken = invite.Token,
2036+
RedirectUri = "http://localhost/callback"
2037+
})
2038+
.StatusCodeShouldBeUnauthorized());
2039+
2040+
// Assert
2041+
var storedUser = await _userRepository.GetByIdAsync(user.Id, options => options.ImmediateConsistency());
2042+
Assert.NotNull(storedUser);
2043+
Assert.False(storedUser.IsActive);
2044+
Assert.False(storedUser.IsEmailAddressVerified);
2045+
Assert.Equal(user.VerifyEmailAddressToken, storedUser.VerifyEmailAddressToken);
2046+
Assert.Equal(user.VerifyEmailAddressTokenExpiration, storedUser.VerifyEmailAddressTokenExpiration);
2047+
Assert.Equal(user.OAuthAccounts.Select(account => (account.Provider, account.ProviderUserId, account.Username)),
2048+
storedUser.OAuthAccounts.Select(account => (account.Provider, account.ProviderUserId, account.Username)));
2049+
Assert.Empty(storedUser.OrganizationIds);
2050+
var tokens = await _tokenRepository.GetByTypeAndUserIdAsync(TokenType.Authentication, user.Id, options => options.ImmediateConsistency());
2051+
Assert.Empty(tokens.Documents);
2052+
var storedOrganization = await _organizationRepository.GetByIdAsync(organization.Id, options => options.ImmediateConsistency());
2053+
Assert.NotNull(storedOrganization);
2054+
Assert.Contains(storedOrganization.Invites, candidate => candidate.Token == invite.Token);
2055+
}
2056+
2057+
[Fact]
2058+
public async Task ExternalLogin_InactiveProviderOwner_DoesNotTransferAccount()
2059+
{
2060+
// Arrange
2061+
const string code = "github-inactive-provider-owner";
2062+
var owner = new User
2063+
{
2064+
EmailAddress = TestOAuthProviderClient.GetEmailAddress(code),
2065+
FullName = "Inactive Owner",
2066+
IsActive = false
2067+
};
2068+
owner.ResetVerifyEmailAddressTokenAndExpiration(TimeProvider);
2069+
owner.AddOAuthAccount("github", code, owner.EmailAddress);
2070+
await _userRepository.AddAsync(owner, options => options.ImmediateConsistency());
2071+
var currentUser = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL);
2072+
Assert.NotNull(currentUser);
2073+
var originalAccounts = currentUser.OAuthAccounts.Select(account => (account.Provider, account.ProviderUserId, account.Username)).ToArray();
2074+
long originalTokenCount = (await _tokenRepository.CountAsync()).Total;
2075+
2076+
// Act
2077+
await SendRequestAsync(request => request
2078+
.Post()
2079+
.AsTestOrganizationUser()
2080+
.AppendPaths("auth", "github")
2081+
.Content(new ExternalAuthInfo
2082+
{
2083+
ClientId = "client-id",
2084+
Code = code,
2085+
RedirectUri = "http://localhost/callback"
2086+
})
2087+
.StatusCodeShouldBeUnauthorized());
2088+
2089+
// Assert
2090+
var storedOwner = await _userRepository.GetByIdAsync(owner.Id, options => options.ImmediateConsistency());
2091+
Assert.NotNull(storedOwner);
2092+
Assert.False(storedOwner.IsEmailAddressVerified);
2093+
Assert.Equal(owner.VerifyEmailAddressToken, storedOwner.VerifyEmailAddressToken);
2094+
Assert.Equal(code, Assert.Single(storedOwner.OAuthAccounts).ProviderUserId);
2095+
var storedCurrentUser = await _userRepository.GetByIdAsync(currentUser.Id, options => options.ImmediateConsistency());
2096+
Assert.NotNull(storedCurrentUser);
2097+
Assert.Equal(originalAccounts, storedCurrentUser.OAuthAccounts.Select(account => (account.Provider, account.ProviderUserId, account.Username)));
2098+
Assert.Equal(originalTokenCount, (await _tokenRepository.CountAsync()).Total);
2099+
}
2100+
2101+
[Fact]
2102+
public async Task GetUserByOAuthProviderAsync_DifferentAccountEntries_DoNotMatch()
2103+
{
2104+
var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL);
2105+
Assert.NotNull(user);
2106+
user.AddOAuthAccount("google", "shared-subject", user.EmailAddress);
2107+
user.AddOAuthAccount("github", "other-subject", user.EmailAddress);
2108+
await _userRepository.SaveAsync(user, o => o.ImmediateConsistency());
2109+
Assert.Null(await _userRepository.GetUserByOAuthProviderAsync("github", "shared-subject"));
2110+
Assert.Equal(user.Id, (await _userRepository.GetUserByOAuthProviderAsync("google", "shared-subject"))!.Id);
2111+
}
2112+
2113+
[Fact]
2114+
public async Task GetUserByOAuthProviderAsync_DuplicateIdentity_RejectsAmbiguousOwner()
2115+
{
2116+
foreach (string email in new[] { SampleDataService.TEST_USER_EMAIL, SampleDataService.TEST_ORG_USER_EMAIL })
2117+
{
2118+
var user = await _userRepository.GetByEmailAddressAsync(email);
2119+
Assert.NotNull(user);
2120+
user.AddOAuthAccount("github", "duplicate-subject", user.EmailAddress);
2121+
await _userRepository.SaveAsync(user, o => o.ImmediateConsistency());
2122+
}
2123+
await Assert.ThrowsAsync<UnauthorizedAccessException>(() => _userRepository.GetUserByOAuthProviderAsync("github", "duplicate-subject"));
2124+
}
2125+
19982126
private async Task AssertExternalLoginAsync(TokenResult? result, string providerName, string providerUserId)
19992127
{
20002128
Assert.NotNull(result);

‎tests/http/users.http‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
@email = admin@exceptionless.test
33
@password = tester
44

5+
# External sign-in returns 401 when the account is unavailable. Existing provider links identify their exact provider and subject.
6+
57
### login to test account
68
# @name login
79
POST {{apiUrl}}/auth/login

0 commit comments

Comments
 (0)