Skip to content

Commit

Permalink
CLDR-18165 cla: sso: additional cleanup, fix tests
Browse files Browse the repository at this point in the history
- add tests
- other cleanup per code review
- support the "employer asserts no rights" round trip
  • Loading branch information
srl295 committed Dec 17, 2024
1 parent bda5c0b commit a7a7828
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 24 deletions.
1 change: 1 addition & 0 deletions tools/cldr-apps/js/src/esm/cldrCla.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as cldrNotify from "../esm/cldrNotify.mjs";
* see ClaSignature.java
* @typedef {Object} ClaSignature
* @property {boolean} corporate true if a corporate signature
* @property {boolean} noRights true if employer asserts no rights
* @property {string} email
* @property {string} employer
* @property {string} name
Expand Down
33 changes: 24 additions & 9 deletions tools/cldr-apps/js/src/views/SignCla.vue
Original file line number Diff line number Diff line change
Expand Up @@ -135,21 +135,21 @@
<!-- don't show the radio button for github or corp -->
<a-form-item v-if="!userGithubSign && !readonlyCla">
<a-radio-group :disabled="!needCla" v-model:value="userSign">
<a-radio :style="radioStyle" :value="2">
<a-radio :style="radioStyle" :value="RADIO_ASSERT_INDIVIDUAL">
I am contributing as an individual because I am self-employed or
unemployed. I have read and agree to the foregoing terms.
</a-radio>
<a-radio :style="radioStyle" :value="3">
<a-radio :style="radioStyle" :value="RADIO_ASSERT_EMPLOYER_NORIGHTS">
I am contributing as an individual because, even though I am
employed, my employer has no rights and claims no rights to my
contributions. I have read and agree to the foregoing terms.
</a-radio>
<a-radio :style="radioStyle" :value="4">
<a-radio :style="radioStyle" :value="RADIO_ASSERT_EMPLOYER_RIGHTS">
I am employed and my employer has or may have rights in my
contributions under my employment agreement and/or the work for
hire doctrine or similar legal principles.
</a-radio>
<a-radio v-if="false" :style="radioStyle" :value="1">
<a-radio v-if="false" :style="radioStyle" :value="RADIO_ASSERT_CORP">
My employer has already signed the CLA and is listed on Unicode’s
<a href="https://www.unicode.org/policies/corporate-cla-list/"
>List of Corporate CLAs</a
Expand All @@ -162,8 +162,8 @@
@click="sign"
v-if="
needCla &&
userSign != 0 &&
userSign != 4 &&
userSign != RADIO_UNSET &&
userSign != RADIO_ASSERT_EMPLOYER_RIGHTS &&
userName &&
userEmail &&
userEmployer &&
Expand All @@ -172,7 +172,7 @@
>
Sign
</button>
<div v-else-if="userSign == 4">
<div v-else-if="userSign == RADIO_ASSERT_EMPLOYER_RIGHTS">
<a-alert
type="error"
message="Please request that your employer sign the Unicode Corporate CLA."
Expand Down Expand Up @@ -227,6 +227,13 @@ import * as cldrStatus from "../esm/cldrStatus.mjs";
import { ref } from "vue";
import claMd from "../md/cla.md";
const RADIO_UNSET = 0;
const RADIO_ASSERT_CORP = 1;
const RADIO_ASSERT_EMPLOYER_RIGHTS = 4; // not allowed for signing.
const RADIO_ASSERT_EMPLOYER_NORIGHTS = 3;
const RADIO_ASSERT_INDIVIDUAL = 2;
const claHtml = marked(claMd);
const user = cldrStatus.getSurveyUser();
Expand Down Expand Up @@ -273,6 +280,7 @@ async function loadData() {
const githubSessionLoad = cldrAuth.getGithubIdFromSession();
const {
corporate,
noRights,
email,
employer,
name,
Expand Down Expand Up @@ -300,7 +308,13 @@ async function loadData() {
userName.value = name;
userEmail.value = email;
userEmployer.value = employer;
userSign.value = corporate ? 1 : 2;
if (corporate) {
userSign.value = RADIO_ASSERT_CORP;
} else if(noRights) {
userSign.value = RADIO_ASSERT_NORIGHTS;
} else {
userSign.value = RADIO_ASSERT_INDIVIDUAL;
}
userGithubSign = github;
}
Expand All @@ -317,7 +331,8 @@ async function sign() {
email: userEmail.value, // unwrap refs
name: userName.value,
employer: userEmployer.value,
corporate: userSign == 1,
corporate: userSign == RADIO_ASSERT_CORP,
noRights: userSign == RADIO_ASSERT_EMPLOYER_NORIGHTS,
});
user.claSigned = true; // update global user obj
needCla.value = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public final class ClaSignature {
public String name;
public String employer; // May be different than org!
public boolean corporate; // signed as corporate
public boolean noRights; // employer claims no rights
public String github; // set if a Github id

@Schema(required = false, description = "Version of CLDR signed in, or * for n/a")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,25 +127,13 @@ public boolean valid() {
return true;
}

// /** Construct the JWT identifying as the client */
// private String getClientJwt() {
// final long now = System.currentTimeMillis();
// return Jwts.builder()
// .setSubject("CLDR SurveyTool")
// .signWith(privateKey)
// .setIssuedAt(new Date(now - (60 * 1000)))
// .setExpiration(new Date(now + (10 * 60 * 1000)))
// .setIssuer(GITHUB_CLIENT_ID)
// .compact();
// }

@Override
public String getLoginUrl(LoginIntent intent) {
if (intent != LoginIntent.cla) {
throw new IllegalArgumentException("LoginIntent must be cla but got " + intent);
}
// Note: does not set redirect URI, that can be done on the front end.
// TODO: add 'state=…' to a temporary token.
// TODO CLDR-18165: add 'state=…' to a temporary token.
return String.format(
"https://github.com/login/oauth/authorize?client_id=%s", GITHUB_CLIENT_ID);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public enum LoginIntent {
public abstract boolean valid();

/**
* Get the URL for the login link. TODO: Probably need more context here eventually.
* Get the URL for the login link.
*
* @param intent what type of login link to generate.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ void testCannedData() throws IOException {
() ->
assertThat(s.keySet())
.containsExactlyInAnyOrder(
"TEST$ind-signer", "TEST$revokr", "TEST$corp-signer"),
"TEST$ind-signer", "TEST$revoker", "TEST$corp-signer"),
() -> assertEquals(SignStatus.signed, s.get("TEST$ind-signer").getSignStatus()),
() -> assertEquals(SignStatus.signed, s.get("TEST$corp-signer").getSignStatus()),
() -> assertEquals(SignStatus.revoked, s.get("TEST$revoker").getSignStatus()));
Expand Down

0 comments on commit a7a7828

Please sign in to comment.