Skip to content

Commit

Permalink
CLDR-18165 cla: sso: cleanup, tests
Browse files Browse the repository at this point in the history
- add tests
- other cleanup per code review
  • Loading branch information
srl295 committed Dec 17, 2024
1 parent fff6b45 commit bda5c0b
Show file tree
Hide file tree
Showing 6 changed files with 268 additions and 61 deletions.
3 changes: 1 addition & 2 deletions tools/cldr-apps/js/src/esm/cldrAuth.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as cldrClient from "../esm/cldrClient.mjs";
import * as cldrStatus from "../esm/cldrStatus.mjs";

/**
* Get the oauth login URL
* Get the oauth login URL. See GithubLoginFactory#getLoginUrl()
* @param {object} o options bag
* @param {string} o.service which service, currently only 'github' is accepted
* @param {string} o.intent what the login URL is used for, currently only 'cla'
Expand All @@ -17,7 +17,6 @@ export async function getLoginUrl(o) {
const client = await cldrClient.getClient();
const { url } = (await client.apis.auth.oauthUrl()).body;
const u = new URL(url);
// TODO: may move this into the GithubLoginFactory#getLoginUrl()
const redir = new URL(window.location);
redir.search = "";
redir.hash = "";
Expand Down
8 changes: 8 additions & 0 deletions tools/cldr-apps/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<version>${assertj-version}</version>
<scope>test</scope>
</dependency>


<!-- icu -->
<dependency>
<groupId>com.ibm.icu</groupId>
Expand Down
194 changes: 136 additions & 58 deletions tools/cldr-apps/src/main/java/org/unicode/cldr/web/ClaGithubList.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.google.gson.stream.JsonReader;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.io.Reader;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
Expand All @@ -16,23 +17,51 @@
import org.unicode.cldr.util.CLDRConfig;
import org.unicode.cldr.util.CLDRConfigImpl;

/**
* Utility class for reading a signatures.json file This file can be downloaded from
* cla-assistant.io and placed in the cldr/ server directory. location can be overridden with the
* CLA_FILE configuration preference. Currently, the file is re-read every time a query occurs, to
* allow the file to be updated.
*
* <p>The simplest way to use this is as follows (given the id “github”): <code>
* ClaGithubList.getInstance().getSignStatus("github")</code>
*/
public class ClaGithubList {
/** Only 'signed' indicates a valid CLA. */
public enum SignStatus {
/** The signature was not found. Used by higher level APIs. */
missing,
/** A good CLA was found. */
signed,
/** The CLA was found, but had been revoked. */
revoked,
};

/** One entry in the CLA file. */
public static final class SignEntry {
/** The GitHub user ID */
public String user_name;

/** Signing date, if any. */
public String signed_at;

/** Revocation date, if any. */
public String revoked_at;

/** User's real name. */
public String name;

/** Type of signature */
public String category;

/** User's employer */
public String employer;

/** User's email */
public String email;

// etc - more fields we don't need.
// Note: There are additional fields in the JSON file which
// are not currently read, and these are ignored.

public SignStatus getSignStatus() {
if (revoked_at != null && !revoked_at.isBlank()) {
Expand Down Expand Up @@ -68,6 +97,7 @@ public Date getSignedAt() {

static final Logger logger = SurveyLog.forClass(ClaGithubList.class);

/** get the singleton instance */
public static final ClaGithubList getInstance() {
return Helper.INSTANCE;
}
Expand All @@ -77,76 +107,46 @@ private static final class Helper {
static final ClaGithubList INSTANCE = new ClaGithubList();
}

static final Gson gson = new Gson();
/** default file path, from configuration */
private final String CLA_FILE;

private final CLDRConfig instance = CLDRConfig.getInstance();

private final String CLA_FILE = instance.getProperty("CLA_FILE", "./signatures.json");
/** full path to .json file */
private final Path claFilePath;

ClaGithubList() {
final File homeFile = ((CLDRConfigImpl) instance).getHomeFile();
claFilePath = new File(homeFile, CLA_FILE).toPath();
logger.info("CLA_FILE=" + claFilePath.toString());
final CLDRConfig instance = CLDRConfig.getInstance();
CLA_FILE = instance.getProperty("CLA_FILE", "./signatures.json");
if (instance instanceof CLDRConfigImpl) {
final File homeFile = ((CLDRConfigImpl) instance).getHomeFile();
claFilePath = new File(homeFile, CLA_FILE).toPath();
logger.fine("CLA_FILE=" + claFilePath.toString());
} else {
claFilePath = null;
logger.fine("claFile path not set, could not get CLDRConfigImpl");
}
}

/**
* @returns the SignEntry for an id, or null if not found
*/
public SignEntry getSignEntry(final String id) {
// Impl: reread each time
Map<String, SignEntry> allSigners = new HashMap<>();
try (final Reader r = new FileReader(claFilePath.toFile(), StandardCharsets.UTF_8);
final JsonReader jr = gson.newJsonReader(r); ) {
jr.beginArray();
while (jr.hasNext()) {
SignEntry e = new SignEntry();
jr.beginObject();
while (jr.hasNext()) {
switch (jr.nextName()) {
case "user_name":
e.user_name = jr.nextString();
break;
case "signed_at":
e.signed_at = jr.nextString();
break;
case "revoked_at":
e.revoked_at = jr.nextString();
break;
case "name":
e.name = jr.nextString();
break;
case "category":
e.category = jr.nextString();
break;
case "employer":
e.employer = jr.nextString();
break;
case "email":
e.email = jr.nextString();
break;
default:
jr.skipValue();
break; // ignore
}
}
jr.endObject();

// the list is in REVERSE order (latest first). So we only track the first
// appearance.
if (!allSigners.containsKey(e.user_name)) {
allSigners.put(e.user_name, e);
// TODO: we could break here.

} // else: ignore, only count first entry
}
jr.endArray();
} catch (Throwable t) {
logger.log(Level.SEVERE, "Trying to read signatures for " + id, t);
Map<String, SignEntry> allSigners;
try {
allSigners = readSigners();
} catch (IOException t) {
logger.log(Level.SEVERE, "Trying to read signatures", t);
return null;
}
logger.info("Read " + allSigners.size() + " signatures");
// Get response
final SignEntry requestedEntry = allSigners.get(id);
return requestedEntry;
}

/**
* This is the simplest and most recommended API for most use cases.
*
* @return the signing status of a GitHub ID, or missing.
*/
public SignStatus getSignStatus(final String id) {
final SignEntry e = getSignEntry(id);
if (e == null) {
Expand All @@ -155,4 +155,82 @@ public SignStatus getSignStatus(final String id) {
return e.getSignStatus();
}
}

/** read the default .json file */
Map<String, SignEntry> readSigners() throws IOException {
if (claFilePath == null) {
throw new NullPointerException(
"CLA_FILE=" + CLA_FILE + " but could not find file path.");
}
return readSigners(claFilePath);
}

/** read a specific path */
Map<String, SignEntry> readSigners(final Path path) throws IOException {
try (final Reader r = new FileReader(path.toFile(), StandardCharsets.UTF_8); ) {
return readSigners(r);
}
}

/** read from a Reader */
Map<String, SignEntry> readSigners(final Reader r) throws IOException {
final Gson gson = new Gson();
Map<String, SignEntry> allSigners = new HashMap<>();
try (final JsonReader jr = gson.newJsonReader(r); ) {
jr.beginArray();
while (jr.hasNext()) {
SignEntry e;
try {
e = parseSignEntry(jr);
// the list is in REVERSE order (latest first). So we only track the first
// appearance.
if (!allSigners.containsKey(e.user_name)) {
allSigners.put(e.user_name, e);
}
// else: ignore, we only count the first (most recent entry).

} catch (Throwable t) {
logger.log(Level.SEVERE, "reading SignEntry - will skip", t);
}
}
jr.endArray();
}
logger.info("Read " + allSigners.size() + " signatures");
return allSigners;
}

private SignEntry parseSignEntry(final JsonReader jr) throws IOException {
final SignEntry e = new SignEntry();
jr.beginObject();
while (jr.hasNext()) {
switch (jr.nextName()) {
case "user_name":
e.user_name = jr.nextString();
break;
case "signed_at":
e.signed_at = jr.nextString();
break;
case "revoked_at":
e.revoked_at = jr.nextString();
break;
case "name":
e.name = jr.nextString();
break;
case "category":
e.category = jr.nextString();
break;
case "employer":
e.employer = jr.nextString();
break;
case "email":
e.email = jr.nextString();
break;
default:
jr.skipValue();
break; // ignore
}
}
jr.endObject();
return e;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package org.unicode.cldr.web;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;

import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.Reader;
import java.util.Map;
import org.junit.jupiter.api.Test;
import org.unicode.cldr.web.ClaGithubList.SignEntry;
import org.unicode.cldr.web.ClaGithubList.SignStatus;

public class TestClaGithubList {
@Test
void testNull() {
assertNotNull(ClaGithubList.getInstance());
}

@Test
void testFailureMode() {
// this will throw in unit test mode
assertThrows(
NullPointerException.class,
() -> ClaGithubList.getInstance().getSignStatus("github"));
}

@Test
void testCannedData() throws IOException {
final ClaGithubList l = new ClaGithubList();
Map<String, SignEntry> m = null;

// directly read
try (final InputStream is =
TestClaGithubList.class
.getResource("data/" + "test-signatures.json")
.openStream();
final Reader r = new InputStreamReader(is); ) {
m = l.readSigners(r);
}

// make this a final for the assertAll()
final Map<String, SignEntry> s = m;

// some basic asserts before we dig into the data
assertNotNull(s);
assertFalse(s.isEmpty());
assertAll(
"ClaGithubList tests",
() -> assertEquals(3, s.size()),
() ->
assertThat(s.keySet())
.containsExactlyInAnyOrder(
"TEST$ind-signer", "TEST$revokr", "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()));
}
}
Loading

0 comments on commit bda5c0b

Please sign in to comment.