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

Make username CLA checks case insignificant #52

Merged
merged 4 commits into from
Jun 21, 2022
Merged

Make username CLA checks case insignificant #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e56469b
Make username CLA checks case insignificant
ambv Jun 20, 2022
e59a8cc
Support old-style pseudo-GH email addresses without numeric IDs
ambv Jun 20, 2022
d76057a
Appease tslint
ambv Jun 20, 2022
e3abe2e
Lowercase emails for existing CLA check
ambv Jun 20, 2022
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
8 changes: 8 additions & 0 deletions dbschema/migrations/00004.edgeql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
CREATE MIGRATION m1mq62xqifdg62gf6oxshidxgnxruewugit5pgsxw6wydiojyytpoq
ONTO m1xz5pf3vdfhz2jg7irvfbvlu3lxnw4yrv2i4ycj3ygrkvgq3mcq6a
{
ALTER TYPE default::ContributorLicenseAgreement {
CREATE PROPERTY normalized_username := (std::str_lower(.username));
CREATE INDEX ON (.normalized_username);
};
};
3 changes: 3 additions & 0 deletions dbschema/structure.esdl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ module default {

property username -> str;

property normalized_username := str_lower(.username);
index on (.normalized_username);

required property creation_time -> datetime {
default := datetime_current();
}
Expand Down
11 changes: 7 additions & 4 deletions service/data/edgedb/cla.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ export class EdgeDBClaRepository
async getClaByEmailAddress(
email: string
): Promise<ContributorLicenseAgreement | null> {
const ghPseudoEmail = /^([0-9]+)\+([^@]+)@users\.noreply\.github\.com$/;
const ghEmailMatches = email.match(ghPseudoEmail);
const ghPseudoEmail = /^(?:[0-9]+)\+([^@]+)@users\.noreply\.github\.com$/;
const ghPseudoEmailOld = /^([^@+]+)@users\.noreply\.github\.com$/;
const ghEmailMatches = (
email.match(ghPseudoEmail) || email.match(ghPseudoEmailOld)
);
let signed_cla = null;
if (ghEmailMatches)
{
Expand All @@ -31,10 +34,10 @@ export class EdgeDBClaRepository
creation_time,
versionId := .agreement_version.id
}
FILTER .username = <str>$0
FILTER .normalized_username = str_lower(<str>$0)
ORDER BY .email
LIMIT 1;`,
[ghEmailMatches[2]]
[ghEmailMatches[1]]
);
});
}
Expand Down
9 changes: 4 additions & 5 deletions service/handlers/sign-cla.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class SignClaHandler {

private getAllAuthors(data: ClaCheckInput): string[] {
if (data.authors) {
return data.authors;
return data.authors.map(email => email.toLowerCase());
}

throw new Error("Missing authors information in state.");
Expand Down Expand Up @@ -130,10 +130,9 @@ class SignClaHandler {

if (!matchingEmails.length) {
// The user who signed in is not among those who contributed to the PR
// NB: this can also happen if the user has a private email address,
// and wants to preserve email privacy when committing.
// As of today, it is unclear how this scenario should be handled.
//
// NB: private email address *are* passed in `getUserEmailAddresses`
// as well. Same for the pseudo-emails Github provides for each user
// in the form 12345678+USERNAME@users.noreply.github.com.
throw new SafeError(
`Thank you for authorizing our application, but the CLA must be signed ` +
`by the users who contributed to the PR. ` +
Expand Down