From 2f28cf33e7793be9cadccf56d2c496389d81d246 Mon Sep 17 00:00:00 2001 From: Amanda Anganes Date: Fri, 3 Aug 2012 16:43:37 -0400 Subject: [PATCH] Changed UserInfo refs in WhitelistedSite to String ids; updated the user approval handler to check if "remember this decision" is checked and only make a new AP if so, and to pull in the scopes selected on the approval page as the saved allowed scopes for that AP. --- .../openid/connect/model/WhitelistedSite.java | 21 ++++++---- .../repository/WhitelistedSiteRepository.java | 5 +-- .../service/WhitelistedSiteService.java | 5 +-- .../db/tables/whitelistedsite.sql | 2 +- .../impl/JpaWhitelistedSiteRepository.java | 7 ++-- .../impl/WhitelistedSiteServiceImpl.java | 5 +-- .../token/JdbcUserApprovalHandler.java | 39 ++++++++++++------- .../webapp/WEB-INF/views/oauth/approve.jsp | 14 ++++--- 8 files changed, 58 insertions(+), 40 deletions(-) diff --git a/openid-connect-common/src/main/java/org/mitre/openid/connect/model/WhitelistedSite.java b/openid-connect-common/src/main/java/org/mitre/openid/connect/model/WhitelistedSite.java index 021382451..a845a5f4a 100644 --- a/openid-connect-common/src/main/java/org/mitre/openid/connect/model/WhitelistedSite.java +++ b/openid-connect-common/src/main/java/org/mitre/openid/connect/model/WhitelistedSite.java @@ -17,6 +17,8 @@ package org.mitre.openid.connect.model; import java.util.Set; +import javax.persistence.Basic; +import javax.persistence.CollectionTable; import javax.persistence.ElementCollection; import javax.persistence.Entity; import javax.persistence.FetchType; @@ -42,7 +44,7 @@ import org.mitre.oauth2.model.ClientDetailsEntity; @NamedQueries({ @NamedQuery(name = "WhitelistedSite.getAll", query = "select w from WhitelistedSite w"), @NamedQuery(name = "WhitelistedSite.getByClientDetails", query = "select w from WhitelistedSite w where w.clientDetails = :clientDetails"), - @NamedQuery(name = "WhitelistedSite.getByUserInfo", query = "select w from WhitelistedSite w where w.creator = :userInfo") + @NamedQuery(name = "WhitelistedSite.getByCreatoruserId", query = "select w from WhitelistedSite w where w.creatorUserId = :userId") }) public class WhitelistedSite { @@ -50,7 +52,7 @@ public class WhitelistedSite { private Long id; // Reference to the admin user who created this entry - private DefaultUserInfo creator; + private String creatorUserId; // which OAuth2 client is this tied to private ClientDetailsEntity clientDetails; @@ -102,6 +104,10 @@ public class WhitelistedSite { * @return the allowedScopes */ @ElementCollection(fetch = FetchType.EAGER) + @CollectionTable( + name="allowed_scopes", + joinColumns=@JoinColumn(name="owner_id") + ) public Set getAllowedScopes() { return allowedScopes; } @@ -113,13 +119,12 @@ public class WhitelistedSite { this.allowedScopes = allowedScopes; } - @ManyToOne - @JoinColumn(name="userinfo_id") - public DefaultUserInfo getCreator() { - return creator; + @Basic + public String getCreatorUserId() { + return creatorUserId; } - public void setCreator(DefaultUserInfo creator) { - this.creator = creator; + public void setCreatorUserId(String creatorUserId) { + this.creatorUserId = creatorUserId; } } diff --git a/openid-connect-common/src/main/java/org/mitre/openid/connect/repository/WhitelistedSiteRepository.java b/openid-connect-common/src/main/java/org/mitre/openid/connect/repository/WhitelistedSiteRepository.java index aa5448ae7..be7f077e5 100644 --- a/openid-connect-common/src/main/java/org/mitre/openid/connect/repository/WhitelistedSiteRepository.java +++ b/openid-connect-common/src/main/java/org/mitre/openid/connect/repository/WhitelistedSiteRepository.java @@ -17,7 +17,6 @@ package org.mitre.openid.connect.repository; import java.util.Collection; -import org.mitre.openid.connect.model.UserInfo; import org.mitre.openid.connect.model.WhitelistedSite; import org.springframework.security.oauth2.provider.ClientDetails; @@ -56,10 +55,10 @@ public interface WhitelistedSiteRepository { /** * Return a collection of the WhitelistedSites created by a given user * - * @param creator the UserInfo representing an admin who may have made some WhitelistedSites + * @param creator the id of the admin who may have created some WhitelistedSites * @return the collection of corresponding WhitelistedSites, if any, or null */ - public Collection getByCreator(UserInfo creator); + public Collection getByCreator(String creatorId); /** * Removes the given IdToken from the repository diff --git a/openid-connect-common/src/main/java/org/mitre/openid/connect/service/WhitelistedSiteService.java b/openid-connect-common/src/main/java/org/mitre/openid/connect/service/WhitelistedSiteService.java index bb4568ae8..9408dffc1 100644 --- a/openid-connect-common/src/main/java/org/mitre/openid/connect/service/WhitelistedSiteService.java +++ b/openid-connect-common/src/main/java/org/mitre/openid/connect/service/WhitelistedSiteService.java @@ -17,7 +17,6 @@ package org.mitre.openid.connect.service; import java.util.Collection; -import org.mitre.openid.connect.model.UserInfo; import org.mitre.openid.connect.model.WhitelistedSite; import org.springframework.security.oauth2.provider.ClientDetails; @@ -56,10 +55,10 @@ public interface WhitelistedSiteService { /** * Return a collection of the WhitelistedSites created by a given user * - * @param creator the UserInfo representing an admin who may have made some WhitelistedSites + * @param creator the user id of an admin who may have made some WhitelistedSites * @return the collection of corresponding WhitelistedSites, if any, or null */ - public Collection getByCreator(UserInfo creator); + public Collection getByCreator(String creatorId); /** * Removes the given WhitelistedSite from the repository diff --git a/openid-connect-server/db/tables/whitelistedsite.sql b/openid-connect-server/db/tables/whitelistedsite.sql index d029d9dbb..88044d985 100644 --- a/openid-connect-server/db/tables/whitelistedsite.sql +++ b/openid-connect-server/db/tables/whitelistedsite.sql @@ -1,5 +1,5 @@ CREATE TABLE whitelistedsite ( id BIGINT AUTO_INCREMENT PRIMARY KEY, - userinfo_id VARCHAR(256), + creatorUserId VARCHAR(256), clientdetails_id VARCHAR(256) ); \ No newline at end of file diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/repository/impl/JpaWhitelistedSiteRepository.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/repository/impl/JpaWhitelistedSiteRepository.java index 22587d1f0..0ced5f0df 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/repository/impl/JpaWhitelistedSiteRepository.java +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/repository/impl/JpaWhitelistedSiteRepository.java @@ -23,7 +23,6 @@ import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; import javax.persistence.TypedQuery; -import org.mitre.openid.connect.model.UserInfo; import org.mitre.openid.connect.model.WhitelistedSite; import org.mitre.openid.connect.repository.WhitelistedSiteRepository; import org.mitre.util.jpa.JpaUtil; @@ -94,9 +93,9 @@ public class JpaWhitelistedSiteRepository implements WhitelistedSiteRepository { @Override @Transactional - public Collection getByCreator(UserInfo creator) { - TypedQuery query = manager.createNamedQuery("WhitelistedSite.getByUserInfo", WhitelistedSite.class); - query.setParameter("userInfo", creator); + public Collection getByCreator(String creatorId) { + TypedQuery query = manager.createNamedQuery("WhitelistedSite.getByCreaterUserId", WhitelistedSite.class); + query.setParameter("userId", creatorId); return query.getResultList(); } diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/service/impl/WhitelistedSiteServiceImpl.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/service/impl/WhitelistedSiteServiceImpl.java index 17a060ea5..4dccb840d 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/service/impl/WhitelistedSiteServiceImpl.java +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/service/impl/WhitelistedSiteServiceImpl.java @@ -17,7 +17,6 @@ package org.mitre.openid.connect.service.impl; import java.util.Collection; -import org.mitre.openid.connect.model.UserInfo; import org.mitre.openid.connect.model.WhitelistedSite; import org.mitre.openid.connect.repository.WhitelistedSiteRepository; import org.mitre.openid.connect.service.WhitelistedSiteService; @@ -86,8 +85,8 @@ public class WhitelistedSiteServiceImpl implements WhitelistedSiteService { } @Override - public Collection getByCreator(UserInfo creator) { - return whitelistedSiteRepository.getByCreator(creator); + public Collection getByCreator(String creatorId) { + return whitelistedSiteRepository.getByCreator(creatorId); } } diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/token/JdbcUserApprovalHandler.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/token/JdbcUserApprovalHandler.java index 2a5168dfd..dc1874324 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/token/JdbcUserApprovalHandler.java +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/token/JdbcUserApprovalHandler.java @@ -17,6 +17,7 @@ package org.mitre.openid.connect.token; import java.util.Collection; import java.util.Date; +import java.util.Map; import java.util.Set; import org.mitre.oauth2.model.ClientDetailsEntity; @@ -113,18 +114,30 @@ public class JdbcUserApprovalHandler implements UserApprovalHandler { if (approved && !authorizationRequest.getApprovalParameters().isEmpty()) { - //TODO: check approval parameters to see if we should store this request or not - - //Make a new AP - ApprovedSite newAP = new ApprovedSite(); - newAP.setAccessDate(new Date()); - String scopes = authorizationRequest.getAuthorizationParameters().get("scope"); - Set allowedScopes = Sets.newHashSet(Splitter.on(" ").split(scopes)); - newAP.setAllowedScopes(allowedScopes); - newAP.setClientDetails((ClientDetailsEntity)client); - newAP.setUserId(userId); - newAP.setCreationDate(new Date()); - approvedSiteService.save(newAP); + //Only store an ApprovedSite if the user has checked "remember this decision": + if (authorizationRequest.getApprovalParameters().get("remember") != null) { + + //Make a new AP + ApprovedSite newAP = new ApprovedSite(); + newAP.setAccessDate(new Date()); + + Set allowedScopes = Sets.newHashSet(); + Map approvalParams = authorizationRequest.getApprovalParameters(); + + for (String key : approvalParams.keySet()) { + if (key.contains("scope")) { + //This is a scope parameter from the approval page. The value sent back should + //be the scope string. + allowedScopes.add(approvalParams.get(key)); + } + } + + newAP.setAllowedScopes(allowedScopes); + newAP.setClientDetails((ClientDetailsEntity)client); + newAP.setUserId(userId); + newAP.setCreationDate(new Date()); + approvedSiteService.save(newAP); + } return true; } @@ -143,7 +156,7 @@ public class JdbcUserApprovalHandler implements UserApprovalHandler { private boolean sitesMatch(ApprovedSite ap, AuthorizationRequest authReq, String userId) { ClientDetails client = clientDetailsService.loadClientByClientId(authReq.getClientId()); - + String scopes = authReq.getAuthorizationParameters().get("scope"); Set allowedScopes = Sets.newHashSet(Splitter.on(" ").split(scopes)); diff --git a/openid-connect-server/src/main/webapp/WEB-INF/views/oauth/approve.jsp b/openid-connect-server/src/main/webapp/WEB-INF/views/oauth/approve.jsp index 2f8b7d074..94e15d888 100644 --- a/openid-connect-server/src/main/webapp/WEB-INF/views/oauth/approve.jsp +++ b/openid-connect-server/src/main/webapp/WEB-INF/views/oauth/approve.jsp @@ -63,15 +63,19 @@
Access to: - + + - + - + - + - + + + +