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.

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

Jump to bottom

Prevent a user with a different email from accepting the team invite #24491

Merged
merged 5 commits into from
May 4, 2023
Merged

Prevent a user with a different email from accepting the team invite #24491

merged 5 commits into from
May 4, 2023

Conversation

jackHay22
Copy link
Contributor

Changes

  • Fixes the case where a logged in user can accept an email invitation even if their email address does not match the address in the invitation

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 2, 2023
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 2, 2023
@jackHay22 jackHay22 changed the title Rrevent a user with a different email from accepting the team invite Prevent a user with a different email from accepting the team invite May 2, 2023
@wxiaoguang
Copy link
Contributor

What's the reason behind this change?

Suppose:

  1. A user uses email my-a@domain.com
  2. They gets an invitation
  3. They changes its email to my-b@domain.com
  4. They wants to accept the invitation

I guess it is expected behavior?

@jackHay22
Copy link
Contributor Author

What's the reason behind this change?

This change fixes the case where a user is logged into a different account than expected and accepts the invitation. Since the invitation is issued for an explicit email, this check should be enforced.

I guess it is expected behavior?

I think the expected behavior is for the user to get a new invitation with the updated email. This allows the invitation to be more explicit about the invitation recipient's address.

@techknowlogick techknowlogick added the outdated/backport/v1.19 This PR should be backported to Gitea 1.19 label May 3, 2023
@techknowlogick techknowlogick added this to the 1.20.0 milestone May 3, 2023
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 3, 2023
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 3, 2023
@jackHay22 jackHay22 force-pushed the jh/fix-prevent-different-email-invite branch from 5acfd74 to bed7370 Compare May 3, 2023 16:35
@lunny
Copy link
Member

lunny commented May 3, 2023

What's the reason behind this change?

Suppose:

1. A user uses email [my-a@domain.com](mailto:my-a@domain.com)

2. They gets an invitation

3. They changes its email to [my-b@domain.com](mailto:my-b@domain.com)

4. They wants to accept the invitation

I guess it is expected behavior?

Since the email is sent to the original email address, we should limit the link just for that email address. If he changed his email, a new invitation link should be sent.

@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 3, 2023
@jackHay22 jackHay22 force-pushed the jh/fix-prevent-different-email-invite branch from bed7370 to 0427c58 Compare May 3, 2023 18:35
@techknowlogick techknowlogick enabled auto-merge (squash) May 3, 2023 19:00
@techknowlogick techknowlogick merged commit 402df1d into go-gitea:main May 4, 2023
@GiteaBot GiteaBot mentioned this pull request May 4, 2023
Closed
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request May 4, 2023
@jackHay22 @GiteaBot
Prevent a user with a different email from accepting the team invite (g…
bf385a1 
…o-gitea#24491)

## Changes
- Fixes the case where a logged in user can accept an email invitation
even if their email address does not match the address in the invitation
@GiteaBot GiteaBot added backport/done All backports for this PR have been created and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels May 4, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request May 4, 2023
@zjjhot
Merge remote-tracking branch 'upstream/main'
5c32bab 
* upstream/main: (65 commits)
  Changelog for 1.19.3 (go-gitea#24495) (go-gitea#24506)
  Use Actions for DB & E2E tests (go-gitea#24494)
  Fix intermittent CI failure in EmptyQueue (go-gitea#23753)
  Prevent a user with a different email from accepting the team invite (go-gitea#24491)
  Fix incorrect webhook time and use relative-time to display it (go-gitea#24477)
  Make Issue/PR/projects more compact, misc CSS tweaks (go-gitea#24459)
  Implement Cargo HTTP index (go-gitea#24452)
  Clean up polluted styles and remove dead CSS code (go-gitea#24497)
  Improve pull request merge box when pull request merged and branch deleted. (go-gitea#24397)
  Fix EasyMDE toolbar (go-gitea#24489)
  Enhance stylelint rule config, remove dead CSS (go-gitea#24472)
  Fix api error message if fork exists (go-gitea#24487)
  Add ntlm authentication support for mail (go-gitea#23811)
  Fix test delivery button in repo webhook settings page (go-gitea#24478)
  Add Debian package registry (go-gitea#24426)
  Enable whitespace rendering on selection in Monaco (go-gitea#24444)
  Replace `N/A` with `-` everywhere (go-gitea#24474)
  Fix invite display (go-gitea#24447)
  [skip ci] Updated translations via Crowdin
  replace PR docker dry run in drone with Actions (go-gitea#24475)
  ...

# Conflicts:
#	templates/base/footer_content.tmpl
@KN4CK3R
Copy link
Member

KN4CK3R commented May 4, 2023

I don't think this was a good change. The original idea was to invite users which do not have an account in the Gitea instance. They receive the mail and register an account. Here they are free to choose an email they want to use. Afterwards the registration flow allows them to join the team.

The feature is not intended to invite existing users because you don't have to invite them, you simply add them as member.

@wxiaoguang
Copy link
Contributor

Agree to revert.

lunny added a commit that referenced this pull request May 5, 2023
@lunny
Revert "Prevent a user with a different email from accepting the team…
f037878 
… invite (#24491)"

This reverts commit 402df1d.
@lunny lunny mentioned this pull request May 5, 2023
Merged
lunny added a commit that referenced this pull request May 5, 2023
@lunny
Revert "Prevent a user with a different email from accepting the team…
3ee7f27 
… invite" (#24531)

Reverts #24491
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny approved these changes

@techknowlogick techknowlogick techknowlogick approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. outdated/backport/v1.19 This PR should be backported to Gitea 1.19 size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
1.20.0
Development

Successfully merging this pull request may close these issues.
6 participants
@jackHay22 @wxiaoguang @lunny @KN4CK3R @techknowlogick @GiteaBot