Skip to content

Verify email ownership for Microsoft authentication #2687

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.

Sign up for GitHub

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

Draft
niemyjski wants to merge 1 commit into
base: issue/external-account-state
Choose a base branch
from
Draft

Verify email ownership for Microsoft authentication #2687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 22 additions & 8 deletions src/Exceptionless.Web/Api/Handlers/AuthHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,11 +569,17 @@ private async Task<Result<TokenResult>> ExternalLoginAsync(ExternalAuthInfo auth
// Ordered user locks also protect transfers and different identities on the same user.
await using var userLock = await lockProvider.AcquireAsync(userKeys, cancellationToken: operationCancellation.Token);
operationCancellation.Token.ThrowIfCancellationRequested();
var user = await FromExternalLoginAsync(userInfo, authInfo.InviteToken, httpContext, existingUser, emailUser, operationCancellation);
var result = await FromExternalLoginAsync(userInfo, authInfo.InviteToken, httpContext, existingUser, emailUser, operationCancellation);
if (!result.IsSuccess)
return Result<TokenResult>.FromResult(result);

var user = result.Value;
if (!String.IsNullOrWhiteSpace(authInfo.InviteToken))
await AddInvitedUserToOrganizationAsync(authInfo.InviteToken, user, httpContext);

if (result.Status == ResultStatus.Created && !user.IsEmailAddressVerified)
await mailer.SendUserEmailVerifyAsync(user);

logger.UserLoggedIn(user.EmailAddress);
return new TokenResult { Token = await GetOrCreateAuthenticationTokenAsync(user) };
}
Expand Down Expand Up @@ -603,13 +609,14 @@ private static void CompleteExternalAccountAdmission(CancellationTokenSource adm
admissionCancellation.CancelAfter(Timeout.InfiniteTimeSpan);
}

private async Task<User> FromExternalLoginAsync(UserInfo userInfo, string? inviteToken, HttpContext httpContext, User? existingUser, User? emailUser, CancellationTokenSource admissionCancellation)
private async Task<Result<User>> FromExternalLoginAsync(UserInfo userInfo, string? inviteToken, HttpContext httpContext, User? existingUser, User? emailUser, CancellationTokenSource admissionCancellation)
{
var cancellationToken = admissionCancellation.Token;
ArgumentException.ThrowIfNullOrWhiteSpace(userInfo.Id);
ArgumentException.ThrowIfNullOrWhiteSpace(userInfo.ProviderName);
ArgumentException.ThrowIfNullOrWhiteSpace(userInfo.Email);

bool isMicrosoft = String.Equals(userInfo.ProviderName, OAuthAccount.KnownProviders.Microsoft, StringComparison.Ordinal);
var accountData = new SettingsDictionary { ["email"] = userInfo.Email };
string? fullName = userInfo.GetFullName();
if (!String.IsNullOrWhiteSpace(fullName))
Expand Down Expand Up @@ -658,7 +665,8 @@ private async Task<User> FromExternalLoginAsync(UserInfo userInfo, string? invit
if (existingUser is not null)
{
CompleteExternalAccountAdmission(admissionCancellation);
existingUser.MarkEmailAddressVerified();
if (!isMicrosoft)
existingUser.MarkEmailAddressVerified();
existingUser.AddOAuthAccount(userInfo.ProviderName, userInfo.Id, userInfo.Email, accountData);
RemoveLegacyMicrosoftOAuthAccounts(existingUser, userInfo.ProviderName);
return await userRepository.SaveAsync(existingUser, o => o.Cache());
Expand All @@ -668,6 +676,10 @@ private async Task<User> FromExternalLoginAsync(UserInfo userInfo, string? invit
if (user is not null && !String.Equals(user.EmailAddress, userInfo.Email.Trim(), StringComparison.OrdinalIgnoreCase))
throw new UnauthorizedAccessException("The user's email address has changed.");

// Microsoft Graph does not provide a mailbox-verification assertion.
if (isMicrosoft && user is not null)
return Result.Forbidden("Sign in to your existing account first, then link Microsoft from your account settings.");

if (user is null)
{
if (!await IsAccountCreationEnabledAsync(inviteToken, cancellationToken))
Expand All @@ -681,16 +693,18 @@ private async Task<User> FromExternalLoginAsync(UserInfo userInfo, string? invit

EnsureUserIsActive(user);
CompleteExternalAccountAdmission(admissionCancellation);
user.MarkEmailAddressVerified();
if (isMicrosoft)
user.ResetVerifyEmailAddressTokenAndExpiration(timeProvider);
else
user.MarkEmailAddressVerified();

user.AddOAuthAccount(userInfo.ProviderName, userInfo.Id, userInfo.Email, accountData);
RemoveLegacyMicrosoftOAuthAccounts(user, userInfo.ProviderName);

if (String.IsNullOrEmpty(user.Id))
await userRepository.AddAsync(user, o => o.Cache());
else
await userRepository.SaveAsync(user, o => o.Cache());
return Result.Created(await userRepository.AddAsync(user, o => o.Cache()));

return user;
return await userRepository.SaveAsync(user, o => o.Cache());
}

private static void EnsureUserIsActive(User user)
Expand Down
5 changes: 4 additions & 1 deletion src/Exceptionless.Web/Api/Handlers/UserHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,10 @@ public async Task<Result<UpdateEmailAddressResult>> Handle(UpdateEmailAddress me

user.ResetPasswordResetToken();
user.EmailAddress = email;
user.IsEmailAddressVerified = user.OAuthAccounts.Any(oa => String.Equals(oa.EmailAddress(), email, StringComparison.InvariantCultureIgnoreCase));
// Microsoft Graph profile email does not establish mailbox ownership.
user.IsEmailAddressVerified = user.OAuthAccounts.Any(oa =>
!String.Equals(oa.Provider, OAuthAccount.KnownProviders.Microsoft, StringComparison.OrdinalIgnoreCase)
&& String.Equals(oa.EmailAddress(), email, StringComparison.InvariantCultureIgnoreCase));
if (user.IsEmailAddressVerified)
user.MarkEmailAddressVerified();
else
Expand Down
95 changes: 78 additions & 17 deletions tests/Exceptionless.Tests/Api/Endpoints/MicrosoftAuthEndpointTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
using Exceptionless.Core.Authorization;
using Exceptionless.Core.Configuration;
using Exceptionless.Core.Extensions;
using Exceptionless.Core.Mail;
using Exceptionless.Core.Models;
using Exceptionless.Core.Repositories;
using Exceptionless.Core.Utility;
using Exceptionless.Tests.Extensions;
using Exceptionless.Tests.Mail;
using Exceptionless.Tests.Utility;
using Exceptionless.Web.Models;
using FluentRest;
using Foundatio.Repositories;
using Microsoft.Extensions.DependencyInjection;
using Xunit;
using ProblemDetails = Microsoft.AspNetCore.Mvc.ProblemDetails;

namespace Exceptionless.Tests.Api.Endpoints;

Expand All @@ -21,6 +25,7 @@ public sealed class MicrosoftAuthEndpointTests : IntegrationTestsBase
private readonly string? _originalMicrosoftSecret;
private readonly IUserRepository _userRepository;
private readonly IOrganizationRepository _organizationRepository;
private readonly CountingMailer _mailer;

public MicrosoftAuthEndpointTests(ITestOutputHelper output, AppWebHostFactory factory) : base(output, factory)
{
Expand All @@ -30,6 +35,14 @@ public MicrosoftAuthEndpointTests(ITestOutputHelper output, AppWebHostFactory fa
_originalMicrosoftSecret = _authOptions.MicrosoftSecret;
_userRepository = GetService<IUserRepository>();
_organizationRepository = GetService<IOrganizationRepository>();
_mailer = GetService<CountingMailer>();
}

protected override void RegisterServices(IServiceCollection services)
{
base.RegisterServices(services);
services.AddSingleton<CountingMailer>();
services.ReplaceSingleton<IMailer>(provider => provider.GetRequiredService<CountingMailer>());
}

protected override async Task ResetDataAsync()
Expand All @@ -39,6 +52,7 @@ protected override async Task ResetDataAsync()
_authOptions.MicrosoftId = "microsoft-client-id";
_authOptions.MicrosoftSecret = "microsoft-client-secret";
await GetService<SampleDataService>().CreateDataAsync();
_mailer.Reset();
}

[Fact]
Expand All @@ -53,86 +67,124 @@ public async Task MicrosoftAsync_AuthenticatedLinkWithDifferentEmail_ReplacesLeg
await _userRepository.SaveAsync(user, o => o.ImmediateConsistency().Cache());

// Act
await SendMicrosoftLoginAsync(code, isAuthenticated: true);
var result = await SendMicrosoftLoginAsync(code, isAuthenticated: true);

// Assert
Assert.NotNull(result);
Assert.False(String.IsNullOrEmpty(result.Token));
var updatedUser = await _userRepository.GetByIdAsync(user.Id);
Assert.NotNull(updatedUser);
Assert.Equal(SampleDataService.TEST_ORG_USER_EMAIL, updatedUser.EmailAddress);
Assert.True(updatedUser.IsEmailAddressVerified);
var account = Assert.Single(updatedUser.OAuthAccounts);
Assert.Equal("microsoft", account.Provider);
Assert.Equal(code, account.ProviderUserId);
Assert.Equal(TestOAuthProviderClient.GetEmailAddress(code), account.Username);
Assert.Equal(0, _mailer.UserEmailVerificationCount);
}

[Fact]
public async Task MicrosoftAsync_ExistingModernIdentity_MarksEmailVerified()
[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task MicrosoftAsync_ExistingModernIdentity_PreservesEmailVerification(bool isEmailVerified)
{
// Arrange
const string code = "existing-microsoft-user";
var user = CreateUser(TestOAuthProviderClient.GetEmailAddress(code));
user.ResetVerifyEmailAddressTokenAndExpiration(TimeProvider);
var user = CreateUser("existing-microsoft-owner@exceptionless.test");
if (!isEmailVerified)
user.ResetVerifyEmailAddressTokenAndExpiration(TimeProvider);
string? verificationToken = user.VerifyEmailAddressToken;
var verificationExpiration = user.VerifyEmailAddressTokenExpiration;
user.AddOAuthAccount(OAuthAccount.KnownProviders.WindowsLive, "legacy-user", user.EmailAddress);
user.AddOAuthAccount(OAuthAccount.KnownProviders.Microsoft, code, user.EmailAddress);
await _userRepository.AddAsync(user, o => o.ImmediateConsistency());

// Act
await SendMicrosoftLoginAsync(code);
var result = await SendMicrosoftLoginAsync(code);

// Assert
Assert.NotNull(result);
var token = await GetService<ITokenRepository>().GetByIdAsync(result.Token);
Assert.NotNull(token);
Assert.Equal(user.Id, token.UserId);
var updatedUser = await _userRepository.GetByIdAsync(user.Id);
Assert.NotNull(updatedUser);
Assert.Equal(user.EmailAddress, updatedUser.EmailAddress);
var account = Assert.Single(updatedUser.OAuthAccounts);
Assert.Equal("microsoft", account.Provider);
Assert.Equal(code, account.ProviderUserId);
Assert.True(updatedUser.IsEmailAddressVerified);
Assert.Null(updatedUser.VerifyEmailAddressToken);
Assert.Equal(TestOAuthProviderClient.GetEmailAddress(code), account.Username);
Assert.Equal(isEmailVerified, updatedUser.IsEmailAddressVerified);
Assert.Equal(verificationToken, updatedUser.VerifyEmailAddressToken);
Assert.Equal(verificationExpiration, updatedUser.VerifyEmailAddressTokenExpiration);
Assert.Equal(0, _mailer.UserEmailVerificationCount);
}

[Fact]
public async Task MicrosoftAsync_ExactEmailMatch_SignsIntoExistingUser()
public async Task MicrosoftAsync_ExactEmailMatch_RequiresAuthenticatedLink()
{
// Arrange
const string code = "matching-email-user";
var user = CreateUser(TestOAuthProviderClient.GetEmailAddress(code));
user.AddOAuthAccount(OAuthAccount.KnownProviders.WindowsLive, "legacy-user", user.EmailAddress);
await _userRepository.AddAsync(user, o => o.ImmediateConsistency());

// Act
await SendMicrosoftLoginAsync(code);
var problem = await SendRequestAsAsync<ProblemDetails>(request => request
.Post()
.AppendPaths("auth", "microsoft")
.Content(new ExternalAuthInfo { ClientId = "microsoft-client-id", Code = code, RedirectUri = "http://localhost" })
.StatusCodeShouldBeForbidden());

// Assert
var updatedUser = await _userRepository.GetByEmailAddressAsync(user.EmailAddress);
Assert.NotNull(updatedUser);
Assert.Equal(user.Id, updatedUser.Id);
var account = Assert.Single(updatedUser.OAuthAccounts);
Assert.Equal("microsoft", account.Provider);
Assert.Equal(code, account.ProviderUserId);
Assert.Equal("windowslive", account.Provider);
Assert.Equal("legacy-user", account.ProviderUserId);
Assert.NotNull(problem);
Assert.Equal("Sign in to your existing account first, then link Microsoft from your account settings.", problem.Title);
Assert.Equal(0, _mailer.UserEmailVerificationCount);
}

[Fact]
public async Task MicrosoftAsync_UnmatchedEmail_CreatesUser()
public async Task MicrosoftAsync_UnmatchedEmail_CreatesUnverifiedUserAndSendsOneVerification()
{
// Arrange
const string code = "new-microsoft-user";
string emailAddress = TestOAuthProviderClient.GetEmailAddress(code);

// Act
await SendMicrosoftLoginAsync(code);
var signup = await SendMicrosoftLoginAsync(code);
await RefreshDataAsync();
var login = await SendMicrosoftLoginAsync(code);

// Assert
Assert.NotNull(signup);
Assert.False(String.IsNullOrEmpty(signup.Token));
Assert.NotNull(login);
Assert.False(String.IsNullOrEmpty(login.Token));
var microsoftUser = await _userRepository.GetByEmailAddressAsync(emailAddress);
Assert.NotNull(microsoftUser);
Assert.True(microsoftUser.IsEmailAddressVerified);
var token = await GetService<ITokenRepository>().GetByIdAsync(signup.Token);
Assert.NotNull(token);
Assert.Equal(microsoftUser.Id, token.UserId);
Assert.False(microsoftUser.IsEmailAddressVerified);
Assert.NotNull(microsoftUser.VerifyEmailAddressToken);
Assert.True(microsoftUser.HasValidVerifyEmailAddressTokenExpiration(TimeProvider));
var account = Assert.Single(microsoftUser.OAuthAccounts);
Assert.Equal("microsoft", account.Provider);
Assert.Equal(code, account.ProviderUserId);
Assert.Equal(1, _mailer.UserEmailVerificationCount);
}

[Fact]
public async Task MicrosoftAsync_MatchingInvitation_CreatesUserAndJoinsOrganization()
{
// Arrange
const string code = "invited-microsoft-user";
_authOptions.EnableAccountCreation = false;
string emailAddress = TestOAuthProviderClient.GetEmailAddress(code);
var organization = (await _organizationRepository.GetAllAsync()).Documents.First();
var invite = new Invite {
Expand All @@ -144,16 +196,20 @@ public async Task MicrosoftAsync_MatchingInvitation_CreatesUserAndJoinsOrganizat
await _organizationRepository.SaveAsync(organization, options => options.ImmediateConsistency());

// Act
await SendMicrosoftLoginAsync(code, inviteToken: invite.Token);
var result = await SendMicrosoftLoginAsync(code, inviteToken: invite.Token);

// Assert
Assert.NotNull(result);
Assert.False(String.IsNullOrEmpty(result.Token));
var user = await _userRepository.GetByEmailAddressAsync(emailAddress);
Assert.NotNull(user);
Assert.True(user.IsEmailAddressVerified);
Assert.Contains(organization.Id, user.OrganizationIds);
var updatedOrganization = await _organizationRepository.GetByIdAsync(organization.Id);
Assert.NotNull(updatedOrganization);
Assert.DoesNotContain(updatedOrganization.Invites, existing => existing.Token == invite.Token);
Assert.Null(user.VerifyEmailAddressToken);
Assert.Equal(0, _mailer.UserEmailVerificationCount);
}

[Fact]
Expand All @@ -163,17 +219,22 @@ public async Task MicrosoftAsync_UnverifiedAuthenticatedUser_CanLinkIdentity()
var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL);
Assert.NotNull(user);
user.ResetVerifyEmailAddressTokenAndExpiration(TimeProvider);
string? verificationToken = user.VerifyEmailAddressToken;
user.AddOAuthAccount(OAuthAccount.KnownProviders.WindowsLive, "legacy-user", user.EmailAddress);
await _userRepository.SaveAsync(user, o => o.ImmediateConsistency().Cache());

// Act
await SendMicrosoftLoginAsync("unverified-link", isAuthenticated: true);
var result = await SendMicrosoftLoginAsync("unverified-link", isAuthenticated: true);

// Assert
Assert.NotNull(result);
Assert.False(String.IsNullOrEmpty(result.Token));
var unchangedUser = await _userRepository.GetByIdAsync(user.Id);
Assert.NotNull(unchangedUser);
Assert.False(unchangedUser.IsEmailAddressVerified);
Assert.Equal(verificationToken, unchangedUser.VerifyEmailAddressToken);
Assert.Contains(unchangedUser.OAuthAccounts, account => account.Provider == "microsoft" && account.ProviderUserId == "unverified-link");
Assert.Equal(0, _mailer.UserEmailVerificationCount);
}

private static User CreateUser(string emailAddress)
Expand Down
13 changes: 7 additions & 6 deletions tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1057,9 +1057,9 @@ public async Task UpdateEmailAddressAsync_ValidEmail_ReturnsResult()
}

[Theory]
[InlineData(OAuthAccount.KnownProviders.Microsoft)]
[InlineData(OAuthAccount.KnownProviders.Google)]
public async Task UpdateEmailAddressAsync_LinkedProviderEmail_MarksAddressVerified(string provider)
[InlineData(OAuthAccount.KnownProviders.Microsoft, false)]
[InlineData(OAuthAccount.KnownProviders.Google, true)]
public async Task UpdateEmailAddressAsync_LinkedProviderEmail_RequiresProofForMicrosoft(string provider, bool isVerified)
{
// Arrange
const string email = "linked-provider@exceptionless.test";
Expand All @@ -1077,12 +1077,13 @@ public async Task UpdateEmailAddressAsync_LinkedProviderEmail_MarksAddressVerifi

// Assert
Assert.NotNull(result);
Assert.True(result.IsVerified);
Assert.Equal(isVerified, result.IsVerified);
var updatedUser = await _userRepository.GetByIdAsync(user.Id);
Assert.NotNull(updatedUser);
Assert.Equal(email, updatedUser.EmailAddress);
Assert.True(updatedUser.IsEmailAddressVerified);
Assert.Null(updatedUser.VerifyEmailAddressToken);
Assert.Equal(isVerified, updatedUser.IsEmailAddressVerified);
Assert.Equal(isVerified, String.IsNullOrEmpty(updatedUser.VerifyEmailAddressToken));
Assert.Equal(!isVerified, updatedUser.HasValidVerifyEmailAddressTokenExpiration(TimeProvider));
}

[Fact]
Expand Down
Loading
Loading