Skip to content

Commit

Permalink
Allow multiple project invites for one user (#1043)
Browse files Browse the repository at this point in the history
* Allow users to accept multiple invitations

Now that we use invitation links for both projects and orgs, it will
start to become rather common for someone to receive an org invitation
and a project invitation within the same few minutes. So we need to
allow users to click on invitation links even if they already have a
Lexbox account.

* Forbid changing email when accepting invite

This prevents stealing someone else's invite by snooping on their email.

* Don't error if same invite accepted twice

If a user clicks the same invitation link twice, he should not get an
error message, he should just be taken to his home page. He's already a
member of the project, so trying to join the project should not error
but just succeed silently.

* Handle edge case of registering existing email

This can only happen if an admin creates the account while the user was
filling out the registration page, so this probably will never happen.
  • Loading branch information
rmunn authored Sep 27, 2024
1 parent 23ab4d5 commit e75aa06
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 13 deletions.
6 changes: 6 additions & 0 deletions Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ tasks:
- wget -c -O {{.DATA_DIR}}/sena-3.zip 'https://drive.google.com/uc?export=download&id=1I-hwc0RHoQqW774gbS5qR-GHa1E7BlsS'
- wget -c -O {{.DATA_DIR}}/empty.zip 'https://drive.google.com/uc?export=download&id=1p73u-AGdSwNkg_5KEv9-4iLRuN-1V-LD'
- wget -c -O {{.DATA_DIR}}/elawa.zip 'https://drive.usercontent.google.com/download?export=download&id=1Jk-eSDho8ATBMS-Kmfatwi-MWQth26ro&confirm=t'
setup-local-env:
cmds:
- echo "HONEYCOMB_API_KEY=__REPLACE__" > deployment/local-dev/local.env
- echo "#OTEL_SDK_DISABLED=true" >> deployment/local-dev/local.env
- echo "GOOGLE_OAUTH_CLIENT_ID=__REPLACE__.apps.googleusercontent.com" >> deployment/local-dev/local.env
- echo "GOOGLE_OAUTH_CLIENT_SECRET=__REPLACE__" >> deployment/local-dev/local.env

# k8s
up:
Expand Down
11 changes: 10 additions & 1 deletion backend/LexBoxApi/Controllers/LoginController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,16 @@ public async Task<ActionResult> LoginRedirect(

await HttpContext.SignInAsync(User,
new AuthenticationProperties { IsPersistent = true });
return Redirect(returnTo);
var destination = ValidateRedirectUrl(returnTo);
return Redirect(destination);
}

private string ValidateRedirectUrl(string url)
{
// Redirect URLs must be relative, to avoid phishing attacks where user is redirected to
// a lookalike site. So we strip off the host if there is one.
var uri = new Uri(url, UriKind.RelativeOrAbsolute);
return uri.IsAbsoluteUri ? uri.PathAndQuery : uri.ToString();
}

[HttpGet("google")]
Expand Down
79 changes: 71 additions & 8 deletions backend/LexBoxApi/Controllers/UserController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,42 @@ await HttpContext.SignInAsync(user.GetPrincipal("Registration"),
return Ok(user);
}

[HttpGet("acceptInvitation")]
[RequireAudience(LexboxAudience.RegisterAccount, true)]
[ProducesResponseType(StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status401Unauthorized)]
public async Task<ActionResult> HandleInviteLink()
{
var user = _loggedInContext.User;
if (user.Email is null)
{
// Malformed JWT, exit early
return Redirect("/login");
}
var dbUser = await _lexBoxDbContext.Users
.Where(u => u.Email == user.Email)
.Include(u => u.Projects)
.Include(u => u.Organizations)
.FirstOrDefaultAsync();
if (dbUser is null)
{
// Go to frontend to fill out registration page
var queryString = QueryString.Create("email", user.Email);
var returnTo = new UriBuilder { Path = "/acceptInvitation", Query = queryString.Value }.Uri.PathAndQuery;
return Redirect(returnTo);
}
else
{
// No need to re-register users that already exist
UpdateUserMemberships(user, dbUser);
await _lexBoxDbContext.SaveChangesAsync();
var loginUser = new LexAuthUser(dbUser);
await HttpContext.SignInAsync(loginUser.GetPrincipal("Invitation"),
new AuthenticationProperties { IsPersistent = true });
return Redirect("/");
}
}

[HttpPost("acceptInvitation")]
[RequireAudience(LexboxAudience.RegisterAccount, true)]
[ProducesResponseType(StatusCodes.Status200OK)]
Expand All @@ -99,18 +135,29 @@ public async Task<ActionResult<LexAuthUser>> AcceptEmailInvitation(RegisterAccou
}

var jwtUser = _loggedInContext.User;
if (jwtUser.Email != accountInput.Email)
{
// Changing email address in invite links is not allowed; this prevents someone from trying to reuse a JWT belonging to somebody else
ModelState.AddModelError<RegisterAccountInput>(r => r.Email, "email address mismatch in invitation link");
return ValidationProblem(ModelState);
}

var hasExistingUser = await _lexBoxDbContext.Users.FilterByEmailOrUsername(accountInput.Email).AnyAsync();
acceptActivity?.AddTag("app.email_available", !hasExistingUser);
if (hasExistingUser)
var userEntity = await _lexBoxDbContext.Users.FindByEmailOrUsername(accountInput.Email);
acceptActivity?.AddTag("app.email_available", userEntity is null);
if (userEntity is null)
{
userEntity = CreateUserEntity(accountInput, jwtUser);
_lexBoxDbContext.Users.Add(userEntity);
}
else
{
// Multiple invitations accepted by the same account should no longer go through this method, so return an error if the account already exists
// That can only happen if an admin created the user's account while the user was still on the registration page: very unlikely
ModelState.AddModelError<RegisterAccountInput>(r => r.Email, "email already in use");
return ValidationProblem(ModelState);
}

var userEntity = CreateUserEntity(accountInput, jwtUser);
acceptActivity?.AddTag("app.user.id", userEntity.Id);
_lexBoxDbContext.Users.Add(userEntity);
await _lexBoxDbContext.SaveChangesAsync();

var user = new LexAuthUser(userEntity);
Expand Down Expand Up @@ -139,16 +186,32 @@ private User CreateUserEntity(RegisterAccountInput input, LexAuthUser? jwtUser,
Locked = false,
CanCreateProjects = false
};
UpdateUserMemberships(jwtUser, userEntity);
return userEntity;
}
private void UpdateUserMemberships(LexAuthUser? jwtUser, User userEntity)
{
// This audience check is redundant now because of [RequireAudience(LexboxAudience.RegisterAccount, true)], but let's leave it in for safety
if (jwtUser?.Audience == LexboxAudience.RegisterAccount && jwtUser.Projects.Length > 0)
{
userEntity.Projects = jwtUser.Projects.Select(p => new ProjectUsers { Role = p.Role, ProjectId = p.ProjectId }).ToList();
foreach (var p in jwtUser.Projects)
{
if (!userEntity.Projects.Exists(proj => proj.ProjectId == p.ProjectId))
{
userEntity.Projects.Add(new ProjectUsers { Role = p.Role, ProjectId = p.ProjectId });
}
}
}
if (jwtUser?.Audience == LexboxAudience.RegisterAccount && jwtUser.Orgs.Length > 0)
{
userEntity.Organizations = jwtUser.Orgs.Select(o => new OrgMember { Role = o.Role, OrgId = o.OrgId }).ToList();
foreach (var o in jwtUser.Orgs)
{
if (!userEntity.Organizations.Exists(org => org.OrgId == o.OrgId))
{
userEntity.Organizations.Add(new OrgMember { Role = o.Role, OrgId = o.OrgId });
}
}
}
return userEntity;
}

[HttpPost("sendVerificationEmail")]
Expand Down
5 changes: 3 additions & 2 deletions backend/LexBoxApi/Services/EmailService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,9 @@ private async Task SendInvitationEmail(
var httpContext = httpContextAccessor.HttpContext;
ArgumentNullException.ThrowIfNull(httpContext);

var queryString = QueryString.Create("email", emailAddress);
var returnTo = new UriBuilder { Path = "/acceptInvitation", Query = queryString.Value }.Uri.PathAndQuery;
var returnTo = _linkGenerator.GetUriByAction(httpContext,
nameof(LexBoxApi.Controllers.UserController.HandleInviteLink),
"User");
var registerLink = _linkGenerator.GetUriByAction(httpContext,
"LoginRedirect",
"Login",
Expand Down
6 changes: 5 additions & 1 deletion frontend/src/lib/components/Users/CreateUser.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
import { createEventDispatcher, onMount } from 'svelte';
import { usernameRe } from '$lib/user';
import { z } from 'zod';
import type { StringifyValues } from '$lib/type.utils';
export let allowUsernames = false;
export let errorOnChangingEmail = '';
export let skipTurnstile = false;
export let submitButtonText = $t('register.button_register');
export let handleSubmit: (password: string, passwordStrength: number, name: string, email: string, locale: string, turnstileToken: string) => Promise<RegisterResponse>;
Expand All @@ -24,6 +26,7 @@
email: string;
};
let turnstileToken = '';
let urlValues = {} as StringifyValues<RegisterPageQueryParams>;
function validateAsEmail(value: string): boolean {
return !allowUsernames || value.includes('@');
Expand All @@ -36,6 +39,7 @@
name: z.string().trim().min(1, $t('register.name_missing')),
email: z.string().trim()
.min(1, $t('project_page.add_user.empty_user_field'))
.refine((value) => !errorOnChangingEmail || !urlValues.email || value == urlValues.email, errorOnChangingEmail)
.refine((value) => !validateAsEmail(value) || isEmail(value), $t('form.invalid_email'))
.refine((value) => validateAsEmail(value) || usernameRe.test(value), $t('register.invalid_username')),
password: passwordFormRules($t),
Expand Down Expand Up @@ -64,7 +68,7 @@
throw new Error('Unknown error, no error from server, but also no user.');
});
onMount(() => { // query params not available during SSR
const urlValues = getSearchParamValues<RegisterPageQueryParams>();
urlValues = getSearchParamValues<RegisterPageQueryParams>();
form.update((form) => {
if (urlValues.name) form.name = urlValues.name;
if (urlValues.email) form.email = urlValues.email;
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lib/i18n/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,7 @@ If you don't see a dialog or already closed it, click the button below:",
"account_exists_email": "An account with this email already exists",
"account_exists_login": "An account with this login/username already exists",
"invalid_username": "Invalid login/username. Only letters, numbers, and underscore (_) characters are allowed.",
"changing_email_not_allowed": "You can't change your email when accepting an invite. You can change your email after your account is registered. Be aware this will invalidate any other invites you have received, so accept them first before changing your email.",
"button_register": "Register",
"label_email": "Email",
"label_email_or_username": "Email or login/username",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@
</script>

<TitlePage title={$t('accept_invitation.title')}>
<CreateUser handleSubmit={acceptInvitation} on:submitted={onSubmit} />
<CreateUser handleSubmit={acceptInvitation} on:submitted={onSubmit} errorOnChangingEmail={$t('register.changing_email_not_allowed')} />
</TitlePage>

0 comments on commit e75aa06

Please sign in to comment.