Hi, Please find the in line comments.
On Wed, Jan 17, 2018 at 10:50 AM, Hasanthi Purnima Dissanayake < [email protected]> wrote: > Hi All, > > In the method [1] which is used to revoke access tokens by resource > owners, it iterates all ACTIVE or EXPIRED access tokens for the particular > client authorized by the user. > > // retrieve all ACTIVE or EXPIRED access tokens for particular client > authorized by this user > > > > Set<AccessTokenDO> accessTokenDOs = OAuthTokenPersistenceFactory.g > etInstance() > > .getAccessTokenDAO().getAccessTokens(appDTO.getOauthConsumerKey(), > > user, userStoreDomain, true); > Inside the foreach the auth cache is cleared for the tokens one by one > which is fetched from the accessTokenDOs. > > for (AccessTokenDO accessTokenDO : accessTokenDOs) { ... > > OAuthUtil.clearOAuthCache(accessTokenDO.getAccessToken()); > > > // retrieve latest access token for particular client, user and scope > combination if its ACTIVE or EXPIRED > scopedToken = OAuthTokenPersistenceFactory.getInstance().getAccessTokenDAO(). > getLatestAccessToken(appDTO.getOauthConsumerKey(), user, userStoreDomain, > Auth2Util.buildScopeString(accessTokenDO.getScope()), true); .... //Revoking > token from database > OAuthTokenPersistenceFactory.getInstance().getAccessTokenDAO() > .revokeAccessTokens(new String[]{scopedToken.getAccessToken()}); > > > ... > } > > Then inside the for each it self it retrieves the latest access token for > the particular client, user and for the particular scope if the token is > ACTIVE or EXPIRED. This token is revoked from the db inside the for each. > > I have two questions here. > > 1. We clear the auth cache based on the access token which is fetched from > accessTokensDo and revoke the scopedToken from the db which can be > different. > > 2. As there are multiple db calls are happening here can't we move the > logic of revoking token from db to out side of the for each as the DAO > method is supporting for batch operations. > No need to get all the access tokens to do cache clean up for the entries against consumer key + authorize user, which can be done as follows. *OAuthUtil.clearOAuthCache(appDTO.getOauthConsumerKey(), user);* Since we remove all the AccessTokenDO entries stored in cache, against all the access tokens as follows, I think we need to retrieve all ACTIVE or EXPIRED access tokens for particular client authorized by the user. *OAuthUtil.clearOAuthCache(accessTokenDO.getAccessToken());* If there are multiple access tokens available in the database for particular client, user and scope combination, actually here we are not revoking the latest access token (ACTIVE or EXPIRED), but revoking multiple entries while iterating through all the access tokens. In order to revoke the latest access token for particular client, user and scope combination, we can use following approach. - Get the scopes for particular client authorized by the user - Iterate through the scopes and revoke the latest access token issued for client, user and scope combination using database batch operation - Remove the corresponding entries in cache for consumer key + authorize user + scope as follows. * OAuthUtil**.clearOAuthCache(appDTO.getOauthConsumerKey(), user, scope));* > > Highly appreciate your feedback on this. > > [1] https://github.com/wso2-extensions/identity-inbound- > auth-oauth/blob/master/components/org.wso2.carbon. > identity.oauth/src/main/java/org/wso2/carbon/identity/ > oauth/OAuthAdminService.java#L627 > > Thanks. > > -- > > Hasanthi Dissanayake > > Senior Software Engineer | WSO2 > > E: [email protected] > M :0718407133| http://wso2.com <http://wso2.com/> > -- Indunil Upeksha Rathnayake Software Engineer | WSO2 Inc Email [email protected] Mobile 0772182255
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
