Issue
18077: Code improvements based on code review
* Added a unique constraint for OpenID identifier
* Changed the module to be translatable
* Removed the silent update of the email when associating account. Will be ask
the user if he wants to update his email account
--- a/modules/org.openbravo.service.integration.openid/src-db/database/model/tables/OBSOID_USER_IDENTIFIER.xml Wed Aug 03 20:05:52 2011 +0200
+++ b/modules/org.openbravo.service.integration.openid/src-db/database/model/tables/OBSOID_USER_IDENTIFIER.xml Wed Aug 03 20:08:22 2011 +0200
@@ -50,6 +50,9 @@
<foreign-key foreignTable="AD_USER" name="OBSOID_USER_OID_AD_USER">
<reference local="AD_USER_ID" foreign="AD_USER_ID"/>
</foreign-key>
+ <unique name="OBSOID_UN_IDENTIFIER">
+ <unique-column name="OPENID_IDENTIFIER"/>
+ </unique>
<check name="OBSOID_USER_OID_ISACTIVE_CHECK"><![CDATA[ISACTIVE IN ('Y', 'N')]]></check>
</table>
</database>
--- a/modules/org.openbravo.service.integration.openid/src-db/database/sourcedata/AD_MODULE.xml Wed Aug 03 20:05:52 2011 +0200
+++ b/modules/org.openbravo.service.integration.openid/src-db/database/sourcedata/AD_MODULE.xml Wed Aug 03 20:08:22 2011 +0200
@@ -14,7 +14,7 @@
<!--FF8080813141B198013141B86DD70003--> <JAVAPACKAGE><![CDATA[org.openbravo.service.integration.openid]]></JAVAPACKAGE>
<!--FF8080813141B198013141B86DD70003--> <LICENSETYPE><![CDATA[OBPL]]></LICENSETYPE>
<!--FF8080813141B198013141B86DD70003--> <AUTHOR><![CDATA[Openbravo S.L.U.]]></AUTHOR>
-<!--FF8080813141B198013141B86DD70003--> <ISTRANSLATIONREQUIRED><![CDATA[N]]></ISTRANSLATIONREQUIRED>
+<!--FF8080813141B198013141B86DD70003--> <ISTRANSLATIONREQUIRED><![CDATA[Y]]></ISTRANSLATIONREQUIRED>
<!--FF8080813141B198013141B86DD70003--> <AD_LANGUAGE><![CDATA[en_US]]></AD_LANGUAGE>
<!--FF8080813141B198013141B86DD70003--> <HASCHARTOFACCOUNTS><![CDATA[N]]></HASCHARTOFACCOUNTS>
<!--FF8080813141B198013141B86DD70003--> <ISTRANSLATIONMODULE><![CDATA[N]]></ISTRANSLATIONMODULE>
--- a/modules/org.openbravo.service.integration.openid/src/org/openbravo/service/integration/openid/OpenIDManager.java Wed Aug 03 20:05:52 2011 +0200
+++ b/modules/org.openbravo.service.integration.openid/src/org/openbravo/service/integration/openid/OpenIDManager.java Wed Aug 03 20:08:22 2011 +0200
@@ -65,9 +65,9 @@
private static ConsumerManager manager;
private static OpenIDManager instance;
- private static Logger log = Logger.getLogger(OpenIDManager.class);
+ private static final Logger log = Logger.getLogger(OpenIDManager.class);
- private static final Map<String, DiscoveryInformation> discoveryInformationMap;
+ private static Map<String, DiscoveryInformation> discoveryInformationMap;
public static final String ATTRIBUTE_EMAIL = "email";
public static final String ATTRIBUTE_FIRSTNAME = "firstName";
@@ -75,13 +75,10 @@
public static final String GOOGLE_OPENID_DISCOVER_URL = "https://www.google.com/accounts/o8/id";
- static {
- discoveryInformationMap = new HashMap<String, DiscoveryInformation>();
- }
-
- public static OpenIDManager getInstance() {
+ public static synchronized OpenIDManager getInstance() {
if (instance == null) {
instance = OBProvider.getInstance().get(OpenIDManager.class);
+ discoveryInformationMap = new HashMap<String, DiscoveryInformation>();
manager = new ConsumerManager();
manager.setAssociations(new InMemoryConsumerAssociationStore());
manager.setNonceVerifier(new InMemoryNonceVerifier(5000));
@@ -150,34 +147,33 @@
u = userCriteria.list().get(0).getUserContact();
}
return u;
+
}
- @SuppressWarnings("unchecked")
public void associateAccount(Identifier oid, HttpServletRequest req, HttpServletResponse resp)
throws Exception {
- Map<String, String> userAttributes = (LinkedHashMap<String, String>) req
- .getAttribute("attributes");
+ // Map<String, String> userAttributes = (LinkedHashMap<String, String>) req
+ // .getAttribute("attributes");
User user = OBDal.getInstance().get(User.class, OBContext.getOBContext().getUser().getId());
-
- if (!userAttributes.get(ATTRIBUTE_EMAIL).equals(user.getEmail())) {
- try {
- user.setEmail(userAttributes.get(ATTRIBUTE_EMAIL));
- OBDal.getInstance().save(user);
- OBDal.getInstance().flush();
- } catch (Exception e) {
- log.error("Error trying to update email for user: " + user.getUsername(), e);
- }
- }
+ // TODO: Ask the user if he wants to update the email account and uncomment this code
+ // if (!userAttributes.get(ATTRIBUTE_EMAIL).equals(user.getEmail())) {
+ // try {
+ // user.setEmail(userAttributes.get(ATTRIBUTE_EMAIL));
+ // OBDal.getInstance().save(user);
+ // OBDal.getInstance().flush();
+ // } catch (Exception e) {
+ // log.error("Error trying to update email for user: " + user.getUsername(), e);
+ // }
+ // }
OBCriteria<OBSOIDUserIdentifier> oidCriteria = OBDal.getInstance().createCriteria(
OBSOIDUserIdentifier.class);
oidCriteria
.add(Restrictions.eq(OBSOIDUserIdentifier.PROPERTY_OPENIDIDENTIFIER, oid.toString()));
- oidCriteria.add(Restrictions.eq(OBSOIDUserIdentifier.PROPERTY_USERCONTACT, user));
if (oidCriteria.count() > 0) {
- log.warn("Account association already exists");
+ log.warn("Account association already exists - OpenID identifier: " + oid.toString());
return;
}
@@ -236,9 +232,6 @@
if (authSuccess.hasExtension(AxMessage.OPENID_NS_AX)) {
FetchResponse fetchResp = (FetchResponse) authSuccess.getExtension(AxMessage.OPENID_NS_AX);
- // List emails = fetchResp.getAttributeValues("email");
- // String email = (String) emails.get(0);
-
List aliases = fetchResp.getAttributeAliases();
Map attributes = new LinkedHashMap();
for (Iterator iter = aliases.iterator(); iter.hasNext();) {