From dea55fb19948e7fa05e4b9369873e96360a5064a Mon Sep 17 00:00:00 2001 From: =?utf8?q?Felix=20D=C3=B6rre?= Date: Sun, 14 Aug 2016 19:20:41 +0200 Subject: [PATCH] upd: narrowing type-safety around Organisation Change-Id: I60b86d46a6a1c580e86826dabc0470524258249b --- .../gigi/dbObjects/CertificateOwner.java | 3 ++ .../cacert/gigi/dbObjects/Organisation.java | 21 ++++++++---- .../cacert/gigi/output/CountrySelector.java | 4 +++ .../account/certs/CertificateRequest.java | 2 +- .../cacert/gigi/pages/orga/CreateOrgForm.java | 9 +---- .../cacert/gigi/pages/orga/ViewOrgPage.java | 2 +- .../gigi/pages/orga/TestOrgManagement.java | 34 +++++++++++-------- 7 files changed, 43 insertions(+), 32 deletions(-) diff --git a/src/org/cacert/gigi/dbObjects/CertificateOwner.java b/src/org/cacert/gigi/dbObjects/CertificateOwner.java index daff8bcb..cc96ade7 100644 --- a/src/org/cacert/gigi/dbObjects/CertificateOwner.java +++ b/src/org/cacert/gigi/dbObjects/CertificateOwner.java @@ -8,6 +8,7 @@ import java.io.Serializable; import java.util.LinkedList; import java.util.List; +import org.cacert.gigi.GigiApiException; import org.cacert.gigi.database.GigiPreparedStatement; import org.cacert.gigi.database.GigiResultSet; @@ -51,6 +52,8 @@ public abstract class CertificateOwner implements IdCachable, Serializable { } else { System.err.print("Malformed cert owner: " + id); } + } catch (GigiApiException e) { + throw new Error(e); } } } diff --git a/src/org/cacert/gigi/dbObjects/Organisation.java b/src/org/cacert/gigi/dbObjects/Organisation.java index bbb3214c..526bf303 100644 --- a/src/org/cacert/gigi/dbObjects/Organisation.java +++ b/src/org/cacert/gigi/dbObjects/Organisation.java @@ -10,6 +10,7 @@ import org.cacert.gigi.GigiApiException; import org.cacert.gigi.database.GigiPreparedStatement; import org.cacert.gigi.database.GigiResultSet; import org.cacert.gigi.dbObjects.Certificate.CertificateStatus; +import org.cacert.gigi.dbObjects.CountryCode.CountryCodeType; import org.cacert.gigi.dbObjects.wrappers.DataContainer; public class Organisation extends CertificateOwner { @@ -53,7 +54,7 @@ public class Organisation extends CertificateOwner { private String name; - private String state; + private CountryCode state; private String province; @@ -69,8 +70,11 @@ public class Organisation extends CertificateOwner { if ( !creator.isInGroup(Group.ORGASSURER)) { throw new GigiApiException("Only Organisation RA Agents may create organisations."); } + if (state == null || state.getCountryCodeType() != CountryCodeType.CODE_2_CHARS) { + throw new GigiApiException("Got country code of illegal type."); + } this.name = name; - this.state = state.getCountryCode(); + this.state = state; this.province = province; this.city = city; this.email = email; @@ -93,10 +97,10 @@ public class Organisation extends CertificateOwner { } } - protected Organisation(GigiResultSet rs) { + protected Organisation(GigiResultSet rs) throws GigiApiException { super(rs.getInt("id")); name = rs.getString("name"); - state = rs.getString("state"); + state = CountryCode.getCountryCode(rs.getString("state"), CountryCodeType.CODE_2_CHARS); province = rs.getString("province"); city = rs.getString("city"); email = rs.getString("contactEmail"); @@ -108,7 +112,7 @@ public class Organisation extends CertificateOwner { return name; } - public String getState() { + public CountryCode getState() { return state; } @@ -206,7 +210,10 @@ public class Organisation extends CertificateOwner { } } - public void updateCertData(String o, CountryCode c, String st, String l) { + public void updateCertData(String o, CountryCode c, String st, String l) throws GigiApiException { + if (c == null || c.getCountryCodeType() != CountryCodeType.CODE_2_CHARS) { + throw new GigiApiException("Got country code of illegal type."); + } for (Certificate cert : getCertificates(false)) { if (cert.getStatus() == CertificateStatus.ISSUED) { cert.revoke(); @@ -221,7 +228,7 @@ public class Organisation extends CertificateOwner { ps.executeUpdate(); } name = o; - state = c.getCountryCode(); + state = c; province = st; city = l; } diff --git a/src/org/cacert/gigi/output/CountrySelector.java b/src/org/cacert/gigi/output/CountrySelector.java index 95e373ba..5bd4b632 100644 --- a/src/org/cacert/gigi/output/CountrySelector.java +++ b/src/org/cacert/gigi/output/CountrySelector.java @@ -32,6 +32,10 @@ public class CountrySelector implements Outputable { public CountrySelector(String name, boolean optional, CountryCode state) { this(name, optional); selected = state == null ? null : state.convertToCountryCodeType(CountryCodeType.CODE_2_CHARS); + if (state.getCountryCodeType() != CountryCodeType.CODE_2_CHARS) { + throw new IllegalArgumentException("Got country code of illegal type."); + } + selected = state; } public void update(HttpServletRequest r) throws GigiApiException { diff --git a/src/org/cacert/gigi/pages/account/certs/CertificateRequest.java b/src/org/cacert/gigi/pages/account/certs/CertificateRequest.java index bae43d58..41c7e84e 100644 --- a/src/org/cacert/gigi/pages/account/certs/CertificateRequest.java +++ b/src/org/cacert/gigi/pages/account/certs/CertificateRequest.java @@ -423,7 +423,7 @@ public class CertificateRequest { if (ctx.getTarget() instanceof Organisation) { Organisation org = (Organisation) ctx.getTarget(); subject.put("O", org.getName()); - subject.put("C", org.getState()); + subject.put("C", org.getState().getCountryCode()); subject.put("ST", org.getProvince()); subject.put("L", org.getCity()); if (ou != null) { diff --git a/src/org/cacert/gigi/pages/orga/CreateOrgForm.java b/src/org/cacert/gigi/pages/orga/CreateOrgForm.java index 5f0b4774..8a9c52db 100644 --- a/src/org/cacert/gigi/pages/orga/CreateOrgForm.java +++ b/src/org/cacert/gigi/pages/orga/CreateOrgForm.java @@ -7,7 +7,6 @@ import javax.servlet.http.HttpServletRequest; import org.cacert.gigi.GigiApiException; import org.cacert.gigi.dbObjects.CountryCode; -import org.cacert.gigi.dbObjects.CountryCode.CountryCodeType; import org.cacert.gigi.dbObjects.Organisation; import org.cacert.gigi.email.EmailProvider; import org.cacert.gigi.localisation.Language; @@ -50,13 +49,7 @@ public class CreateOrgForm extends Form { result = t; o = t.getName(); - CountryCode orgState = null; - try { - orgState = CountryCode.getCountryCode(t.getState(), CountryCodeType.CODE_2_CHARS); - } catch (GigiApiException e) { - throw new Error(e); - } - cs = new CountrySelector("C", false, orgState); + cs = new CountrySelector("C", false, t.getState()); st = t.getProvince(); l = t.getCity(); diff --git a/src/org/cacert/gigi/pages/orga/ViewOrgPage.java b/src/org/cacert/gigi/pages/orga/ViewOrgPage.java index 98f76353..d5839013 100644 --- a/src/org/cacert/gigi/pages/orga/ViewOrgPage.java +++ b/src/org/cacert/gigi/pages/orga/ViewOrgPage.java @@ -132,7 +132,7 @@ public class ViewOrgPage extends Page { Organisation org = orgas[count++]; vars.put("id", Integer.toString(org.getId())); vars.put("name", org.getName()); - vars.put("country", org.getState()); + vars.put("country", org.getState().getCountryCode()); return true; } }; diff --git a/tests/org/cacert/gigi/pages/orga/TestOrgManagement.java b/tests/org/cacert/gigi/pages/orga/TestOrgManagement.java index c8f3d119..dc922907 100644 --- a/tests/org/cacert/gigi/pages/orga/TestOrgManagement.java +++ b/tests/org/cacert/gigi/pages/orga/TestOrgManagement.java @@ -149,7 +149,7 @@ public class TestOrgManagement extends OrgTest { Organisation o1 = createUniqueOrg(); o1.updateCertData("name", CountryCode.getCountryCode("DE", CountryCodeType.CODE_2_CHARS), DIFFICULT_CHARS, "Köln"); assertEquals("name", o1.getName()); - assertEquals("DE", o1.getState()); + assertEquals("DE", o1.getState().getCountryCode()); assertEquals(DIFFICULT_CHARS, o1.getProvince()); assertEquals("Köln", o1.getCity()); o1.delete(); @@ -179,25 +179,25 @@ public class TestOrgManagement extends OrgTest { String s128 = str128; String s129 = str128 + "a"; - assertNull(upCertData(o1, o1.getName(), o1.getState(), o1.getProvince(), o1.getCity())); + assertNull(upCertData(o1, o1.getName(), null, o1.getProvince(), o1.getCity())); // test organisation name - assertNotNull(upCertData(o1, "", o1.getState(), o1.getProvince(), o1.getCity())); - assertNull(upCertData(o1, "A", o1.getState(), o1.getProvince(), o1.getCity())); - assertNull(upCertData(o1, s64, o1.getState(), o1.getProvince(), o1.getCity())); - assertNotNull(upCertData(o1, s65, o1.getState(), o1.getProvince(), o1.getCity())); + assertNotNull(upCertData(o1, "", null, o1.getProvince(), o1.getCity())); + assertNull(upCertData(o1, "A", null, o1.getProvince(), o1.getCity())); + assertNull(upCertData(o1, s64, null, o1.getProvince(), o1.getCity())); + assertNotNull(upCertData(o1, s65, null, o1.getProvince(), o1.getCity())); // test state - assertNotNull(upCertData(o1, o1.getName(), o1.getState(), se, o1.getCity())); - assertNull(upCertData(o1, o1.getName(), o1.getState(), "A", o1.getCity())); - assertNull(upCertData(o1, o1.getName(), o1.getState(), s128, o1.getCity())); - assertNotNull(upCertData(o1, o1.getName(), o1.getState(), s129, o1.getCity())); + assertNotNull(upCertData(o1, o1.getName(), null, se, o1.getCity())); + assertNull(upCertData(o1, o1.getName(), null, "A", o1.getCity())); + assertNull(upCertData(o1, o1.getName(), null, s128, o1.getCity())); + assertNotNull(upCertData(o1, o1.getName(), null, s129, o1.getCity())); // test town - assertNotNull(upCertData(o1, o1.getName(), o1.getState(), o1.getProvince(), se)); - assertNull(upCertData(o1, o1.getName(), o1.getState(), o1.getProvince(), "A")); - assertNull(upCertData(o1, o1.getName(), o1.getState(), o1.getProvince(), s128)); - assertNotNull(upCertData(o1, o1.getName(), o1.getState(), o1.getProvince(), s129)); + assertNotNull(upCertData(o1, o1.getName(), null, o1.getProvince(), se)); + assertNull(upCertData(o1, o1.getName(), null, o1.getProvince(), "A")); + assertNull(upCertData(o1, o1.getName(), null, o1.getProvince(), s128)); + assertNotNull(upCertData(o1, o1.getName(), null, o1.getProvince(), s129)); // test country assertNotNull(upCertData(o1, o1.getName(), "", o1.getProvince(), o1.getCity())); @@ -236,7 +236,8 @@ public class TestOrgManagement extends OrgTest { * @param o * the new name * @param c - * the new country + * the new country or null to keep the current + * country. * @param province * the new "province/state" * @param ct @@ -244,6 +245,9 @@ public class TestOrgManagement extends OrgTest { * @return an error message or null */ private String upCertData(Organisation o1, String o, String c, String province, String ct) throws IOException, MalformedURLException, UnsupportedEncodingException { + if (c == null) { + c = o1.getState().getCountryCode(); + } return executeBasicWebInteraction(cookie, ViewOrgPage.DEFAULT_PATH + "/" + o1.getId(), "action=updateCertificateData&O=" + o + "&C=" + c + "&ST=" + province + "&L=" + ct, 0); } -- 2.39.2