[server] store separate email used for commits (GitHub and GitLab) #4115

philschatz · 2021-04-29T23:11:01Z

GitLab and GitHub both have a way to specify an email address for commits that is different than the primary email used for a user account.

GitLab contains a field for the commit email and while GitHub does not provide a similar field in their API, their Desktop client constructs this email address

🤞 😅 that this works.

Fixes #387, #2007, #3186, #4101

components/server/src/github/github-auth-provider.ts

components/server/src/auth/auth-provider.ts

AlexTugarev · 2021-05-04T09:37:15Z

/werft run

👍 started the job as gitpod-build-ps-commit-email-fork.0

philschatz · 2021-05-05T15:41:45Z

Thank you for the suggestions! I updated the PR with them but had trouble connecting to https://ps-commit-email-fork.staging.gitpod-dev.com/workspaces/ to see if it works. Is there something I can set up?

AlexTugarev · 2021-05-06T09:38:19Z

/werft run

👍 started the job as gitpod-build-ps-commit-email-fork.1

AlexTugarev · 2021-05-06T09:41:16Z

@philschatz, thanks for the update. I've triggered a a new build on werft. Our public werft is only listening to GItpod devs. I keep an eye open on the PR to trigger it, but if you won't see a new update timely, feel free to ping me 🙏🏻

AlexTugarev · 2021-05-06T11:26:42Z

@philschatz, we forgot to add a db migration.

I'm happy to add this to this PR. Just, internally we've a discussion on how to add columns to db tables like d_b_identity. 'Keep you posted.

philschatz · 2021-05-18T15:11:39Z

Hello @AlexTugarev 👋 . Is there any decision on how to do the db migration?

AlexTugarev · 2021-05-19T12:07:47Z

Hi @philschatz! Sorry for the delay.
A nice option would be to introduce Identity.emails: { address:string, type: string }. As we need to be careful to not introduce expensive blocking DB migration on the production environment, it would be great to rename d_b_identity.tokens in a migration and reuse it, because this isn't in use anymore.
I'll try to make a PR against yours.

corneliusludmann · 2021-06-23T09:40:32Z

Hi @philschatz!

Sorry, that it took so long to get feedback. We need to get better at this.

I would like to get your great contribution into the June release which will be deployed by the end of this month. I rebased your contribution and added the proposed changes from Alex in this branch (→ diff).

We have now 2 options:

We could use my branch with your commits and I'll create a PR on behalf of both of us. In that case, no action is required on your end.
You could update your PR with the changes in my PR.

Your call!

After that, I'll take care that we get a review soon.

What do you think?

corneliusludmann · 2021-06-23T09:47:04Z

There is one more thing, @philschatz: For legal reasons, we need a signed contributor license agreement (CLA) from each contributor. @meysholdt will get back to you on this.

philschatz · 2021-06-29T17:55:56Z

Hello @corneliusludmann ! I somehow missed your comment on options but did sign the CLA and rebased this Pull Request with your commits.

Please let me know if there is anything else you need to get this merged (yay!)

corneliusludmann · 2021-06-30T07:25:37Z

Awesome! Thanks a lot!

@AlexTugarev Do you have time to review this PR? Since I've contributed as well I need a second pair of eyes.

AlexTugarev · 2021-06-30T08:52:03Z

components/server/src/auth/auth-provider.ts

@@ -65,6 +65,7 @@ export interface AuthUser {
    readonly authId: string;
    readonly authName: string;
    readonly primaryEmail: string;
+    readonly additionalEmails?: { address: string, type: string }[];


nit: could be typed as Email as well, right?

👍 Yep! I believe this is now resolved.

AlexTugarev · 2021-06-30T08:57:43Z

components/server/src/github/github-auth-provider.ts

@@ -98,26 +98,35 @@ export class GitHubAuthProvider extends GenericAuthProvider {
                .map((s: string) => s.trim())
            );

-            const filterPrimaryEmail = (emails: typeof userEmails) => {
+            const blockPassListEmail = ((emails: typeof userEmails) => {


I think this should be moved to UserService.handleNewUser, cf. https://github.com/gitpod-io/gitpod/blob/main/components/server/src/user/user-service.ts#L140
This way, we do not need to repeat filtering.

When we store all verified emails (as suggested below) we can drop this here because it is already in UserService.handleNewUser. See:

https://github.com/philschatz/gitpod/blob/88af0c51bf7c29b55e510c24d7c825aaa28b3254/components/server/src/user/user-service.ts#L143-L144

Otherwise, this filter would be necessary to store just the needed email addresses.

👍 I believe this is now resolved.

AlexTugarev · 2021-06-30T09:05:09Z

components/server/src/github/github-auth-provider.ts


            return <AuthUserSetup>{
                authUser: {
                    authId: String(id),
                    authName: login,
                    avatarUrl: avatar_url,
                    name,
-                    primaryEmail: filterPrimaryEmail(userEmails)
+                    primaryEmail: primary.email,
+                    additionalEmails,


I think we should add all verified email addresses to additionalEmails here. This could also be of use to improve the signup process.

The proxy is marked as type=commit, which is also available in the GitLab case. 👍🏻

For the git config we would use identity.additionalEmails.find(isCommitEmail) || identity.primaryEmail then.

OK, then I would suggest adding all verified and non-private (public) email addresses.

The commit email gets the type commit (as it is now) and all others get the type other.

👍 All verified, non-private, and non-primary emails should now be in the additionalEmails array.

AlexTugarev · 2021-06-30T09:06:07Z

CHANGELOG.md

@@ -4,6 +4,7 @@ All notable changes to this project will be documented in this file.

 ## June 2021

+- Support custom commit email address for GitHub and GitLab (keep emai private). Thanks to [@philschatz](https://github.com/philschatz) for the contribution! ([#4115](https://github.com/gitpod-io/gitpod/pull/4115))


Also: Since we don't get this into June release, we should move this to July.

components/gitpod-db/src/typeorm/migration/1624290876856-IdentityTokensToEmails.ts

geropl · 2021-06-30T09:29:32Z

components/gitpod-protocol/src/protocol.ts

@@ -323,6 +320,21 @@ export namespace Identity {
    }
 }

+export interface Email {
+    address: string;
+    type: string;


We definitely should make type typesafe by having a export type EmailType = "commit" | "block_pass" (or similar).

👍 type is now COMMIT | OTHER since the block_pass logic is handled in user-service.ts.

corneliusludmann · 2021-06-30T12:16:51Z

Thanks, Alex and Gero for the feedback.

@philschatz Please let me know if I should prepare a commit with the suggested changes (I would be happy to do so) or if you would like to do this. Your call! 🙂

corneliusludmann · 2021-07-06T11:32:07Z

/werft run

👍 started the job as gitpod-build-ps-commit-email-fork.2

corneliusludmann · 2021-07-08T08:45:04Z

Hi @philschatz! Thanks for updating this PR. We are almost there! 🚀

I reviewed and tested your changes: It looks great! Unfortunately, this branch does not build because we need to rebase it on top of main and it has also merge conflicts. The good news is: I already did it in this branch and it works pretty well. I also took the liberty to squash your commits into one to have a cleaner git history.

Feel free to take the branch clu/commit-email for this PR and we are just a click away from merging this into main and getting this in production soon.

Thanks again for taking the initiative to contribute to this change and thanks for your patience. I really looking forward to having this great feature in production!

corneliusludmann

Thanks to you, @philschatz, we finally have this frequently requested feature in Gitpod! 🥳

AlexTugarev · 2021-07-08T15:23:23Z

🎉 Let's verify on staging that the migration and transformation works 👍🏻

Thanks @corneliusludmann and @philschatz!

philschatz · 2021-07-08T15:25:40Z

Thanks everyone for the help and feedback!

Also, I'm excited to try deploying 0.9.0 on my little k3s at home and being able to edit & test more contributions!

philschatz mentioned this pull request May 3, 2021

Support custom email addresses #4101

Closed

AlexTugarev self-requested a review May 4, 2021 05:34

AlexTugarev reviewed May 4, 2021

View reviewed changes

components/server/src/github/github-auth-provider.ts Outdated Show resolved Hide resolved

AlexTugarev reviewed May 4, 2021

View reviewed changes

components/server/src/auth/auth-provider.ts Outdated Show resolved Hide resolved

AlexTugarev added the community-contribution label May 4, 2021

AlexTugarev mentioned this pull request May 19, 2021

[github-auth-provider] Prefer pass list email over primary email #4237

Merged

philschatz force-pushed the ps/commit-email branch from 582debc to 88af0c5 Compare June 29, 2021 17:52

corneliusludmann requested a review from AlexTugarev June 30, 2021 07:25

AlexTugarev reviewed Jun 30, 2021

View reviewed changes

components/gitpod-db/src/typeorm/migration/1624290876856-IdentityTokensToEmails.ts Show resolved Hide resolved

geropl reviewed Jun 30, 2021

View reviewed changes

philschatz requested a review from a team as a code owner July 6, 2021 04:25

philschatz requested a review from a team July 6, 2021 04:25

corneliusludmann removed request for a team July 6, 2021 09:48

philschatz and others added 2 commits July 7, 2021 14:03

[server] support separate commit email

9fcf39a

[typeorm] Catch JSON error in SIMPLE_JSON transformer

865cd57

philschatz force-pushed the ps/commit-email branch from 68ffa38 to 865cd57 Compare July 8, 2021 14:22

corneliusludmann approved these changes Jul 8, 2021

View reviewed changes

corneliusludmann merged commit 1e78c6c into gitpod-io:main Jul 8, 2021

corneliusludmann mentioned this pull request Jul 8, 2021

Make account information editable #4629

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[server] store separate email used for commits (GitHub and GitLab) #4115

[server] store separate email used for commits (GitHub and GitLab) #4115

philschatz commented Apr 29, 2021

AlexTugarev commented May 4, 2021 •

edited by werft-gitpod-dev-com bot

Loading

philschatz commented May 5, 2021

AlexTugarev commented May 6, 2021 •

edited by werft-gitpod-dev-com bot

Loading

AlexTugarev commented May 6, 2021 •

edited by werft-gitpod-dev-com bot

Loading

AlexTugarev commented May 6, 2021

philschatz commented May 18, 2021

AlexTugarev commented May 19, 2021

corneliusludmann commented Jun 23, 2021 •

edited by werft-gitpod-dev-com bot

Loading

corneliusludmann commented Jun 23, 2021

philschatz commented Jun 29, 2021

corneliusludmann commented Jun 30, 2021

AlexTugarev Jun 30, 2021

philschatz Jul 6, 2021

AlexTugarev Jun 30, 2021

corneliusludmann Jun 30, 2021

philschatz Jul 6, 2021

AlexTugarev Jun 30, 2021

corneliusludmann Jun 30, 2021

philschatz Jul 6, 2021

AlexTugarev Jun 30, 2021

corneliusludmann Jun 30, 2021

geropl Jun 30, 2021

philschatz Jul 6, 2021

corneliusludmann commented Jun 30, 2021

corneliusludmann commented Jul 6, 2021 •

edited by werft-gitpod-dev-com bot

Loading

corneliusludmann commented Jul 8, 2021

corneliusludmann left a comment

AlexTugarev commented Jul 8, 2021 •

edited by werft-gitpod-dev-com bot

Loading

philschatz commented Jul 8, 2021

		@@ -4,6 +4,7 @@ All notable changes to this project will be documented in this file.

		## June 2021

		- Support custom commit email address for GitHub and GitLab (keep emai private). Thanks to [@philschatz](https://github.com/philschatz) for the contribution! ([#4115](https://github.com/gitpod-io/gitpod/pull/4115))

[server] store separate email used for commits (GitHub and GitLab) #4115

[server] store separate email used for commits (GitHub and GitLab) #4115

Conversation

philschatz commented Apr 29, 2021

AlexTugarev commented May 4, 2021 • edited by werft-gitpod-dev-com bot Loading

philschatz commented May 5, 2021

AlexTugarev commented May 6, 2021 • edited by werft-gitpod-dev-com bot Loading

AlexTugarev commented May 6, 2021 • edited by werft-gitpod-dev-com bot Loading

AlexTugarev commented May 6, 2021

philschatz commented May 18, 2021

AlexTugarev commented May 19, 2021

corneliusludmann commented Jun 23, 2021 • edited by werft-gitpod-dev-com bot Loading

corneliusludmann commented Jun 23, 2021

philschatz commented Jun 29, 2021

corneliusludmann commented Jun 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

corneliusludmann commented Jun 30, 2021

corneliusludmann commented Jul 6, 2021 • edited by werft-gitpod-dev-com bot Loading

corneliusludmann commented Jul 8, 2021

corneliusludmann left a comment

Choose a reason for hiding this comment

AlexTugarev commented Jul 8, 2021 • edited by werft-gitpod-dev-com bot Loading

philschatz commented Jul 8, 2021

AlexTugarev commented May 4, 2021 •

edited by werft-gitpod-dev-com bot

Loading

AlexTugarev commented May 6, 2021 •

edited by werft-gitpod-dev-com bot

Loading

AlexTugarev commented May 6, 2021 •

edited by werft-gitpod-dev-com bot

Loading

corneliusludmann commented Jun 23, 2021 •

edited by werft-gitpod-dev-com bot

Loading

corneliusludmann commented Jul 6, 2021 •

edited by werft-gitpod-dev-com bot

Loading

AlexTugarev commented Jul 8, 2021 •

edited by werft-gitpod-dev-com bot

Loading