Skip to content

Commit 4929c7d

Browse files
committed
Fix emailDomains and tests
Fix emailDomains to be empty string instead of null Add/update tests for new constructor
1 parent cc9dae5 commit 4929c7d

File tree

2 files changed

+18
-16
lines changed

2 files changed

+18
-16
lines changed

src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public GithubSecurityRealm(String githubWebUri,
144144
this.clientID = Util.fixEmptyAndTrim(clientID);
145145
setClientSecret(Util.fixEmptyAndTrim(clientSecret));
146146
this.oauthScopes = Util.fixEmptyAndTrim(oauthScopes);
147-
this.emailDomains = Util.fixEmptyAndTrim(emailDomains);
147+
this.emailDomains = emailDomains.trim();
148148
this.forceGithubEmail = forceGithubEmail;
149149
}
150150

@@ -280,11 +280,9 @@ public void marshal(Object source, HierarchicalStreamWriter writer,
280280
writer.setValue(realm.getOauthScopes());
281281
writer.endNode();
282282

283-
if (realm.getEmailDomains() != null) {
284-
writer.startNode("emailDomains");
285-
writer.setValue(realm.getEmailDomains());
286-
writer.endNode();
287-
}
283+
writer.startNode("emailDomains");
284+
writer.setValue(realm.getEmailDomains());
285+
writer.endNode();
288286

289287
writer.startNode("forceGithubEmail");
290288
//TODO: Is there a better way to do this?

src/test/java/org/jenkinsci/plugins/GithubSecurityRealmTest.java

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,31 +43,35 @@ public void testEquals_true() {
4343
GithubSecurityRealm a = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org");
4444
GithubSecurityRealm b = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org");
4545
GithubSecurityRealm c = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org", "", false);
46-
GithubSecurityRealm d = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org", "", false);
46+
GithubSecurityRealm d = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org", "example.com", false);
47+
GithubSecurityRealm e = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org", "example.com", false);
4748
assertTrue(a.equals(b));
4849
assertTrue(b.equals(c));
4950
assertTrue(c.equals(b));
50-
assertTrue(c.equals(d));
51+
assertTrue(d.equals(e));
52+
assertTrue(e.equals(d));
5153
}
5254

5355
@Test
5456
public void testEquals_false() {
55-
GithubSecurityRealm a = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org");
56-
GithubSecurityRealm b = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,repo");
57-
GithubSecurityRealm c = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org", "", false);
58-
GithubSecurityRealm d = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,repo", "", false);
57+
GithubSecurityRealm a = new GithubSecurityRealm("http:jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org");
58+
GithubSecurityRealm b = new GithubSecurityRealm("http:jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,repo");
59+
GithubSecurityRealm c = new GithubSecurityRealm("http:jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org", "", false);
60+
GithubSecurityRealm d = new GithubSecurityRealm("http:jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,repo", "", false);
61+
GithubSecurityRealm e = new GithubSecurityRealm("http:jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,repo", "example.com", false);
5962
assertFalse(a.equals(b));
6063
assertFalse(a.equals(d));
6164
assertFalse(a.equals(""));
6265
assertFalse(c.equals(b));
6366
assertFalse(c.equals(d));
6467
assertFalse(c.equals(""));
68+
assertFalse(d.equals(e));
6569
}
6670

6771
@Test
6872
public void testHasScope_true() {
69-
GithubSecurityRealm a = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,user,user:email", "", false);
70-
GithubSecurityRealm b = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,user,user:email");
73+
GithubSecurityRealm a = new GithubSecurityRealm("http:jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,user,user:email");
74+
GithubSecurityRealm b = new GithubSecurityRealm("http:jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,user,user:email", "example.com", false);
7175
assertTrue(a.hasScope("user"));
7276
assertTrue(a.hasScope("read:org"));
7377
assertTrue(a.hasScope("user:email"));
@@ -78,8 +82,8 @@ public void testHasScope_true() {
7882

7983
@Test
8084
public void testHasScope_false() {
81-
GithubSecurityRealm a = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,user,user:email");
82-
GithubSecurityRealm b = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,user,user:email", "", false);
85+
GithubSecurityRealm a = new GithubSecurityRealm("http:jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,user,user:email");
86+
GithubSecurityRealm b = new GithubSecurityRealm("http:jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,user,user:email", "example.com", false);
8387
assertFalse(a.hasScope("somescope"));
8488
assertFalse(b.hasScope("somescope"));
8589
}

0 commit comments

Comments
 (0)