Had a look at the code and looks like we are doing a redundant check in the two methods. Ideally, we should only have this logic in isAuthorizedClient() method.
IMO your analysis is correct and we should remove the redundant logic. We have similar methods in AuthorizationGrantHandler interface as well. There, 1. isAuthorizedClient() method validates whether the client is using an allowed grant type 2. authorizeAccessDelegation() is used to fire callback handlers to achieve scope validation logic etc. (ie. to validate whether the authorized user is allowed to request the particular scope)[2] [1] https://github.com/wso2-extensions/identity-inbound-auth-oauth/blob/c8683a407b22327fb57492dda313ca665d0d29f9/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AbstractAuthorizationGrantHandler.java#L675-L675 [2] https://github.com/wso2-extensions/identity-inbound-auth-oauth/blob/c8683a407b22327fb57492dda313ca665d0d29f9/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AbstractAuthorizationGrantHandler.java#L605-L605 Thanks, Farasath Farasath Ahamed Software Engineer, WSO2 Inc.; http://wso2.com Mobile: +94777603866 Blog: blog.farazath.com Twitter: @farazath619 <https://twitter.com/farazath619> <http://wso2.com/signature> On Thu, Nov 2, 2017 at 2:54 PM, Danushka Fernando <[email protected]> wrote: > Hi All > When access token, id token, auth code or open id token is requested, it > will go through AuthorizationHandlerManager[1] class to authorize the > client. There are three authorization steps [2]. > > 1. First check is isAuthorized check. Here it checks whether its > requesting a token or a code and according to that it will check implicit > or code grant types are allowed for the application and returns true of > false.[3] > 2. Second check is validateAccessDelegation check. Here also it checks > the request type and will check allowance of implicit or code grant types > and returns true or false.[4] > 3. Third is scope validation > > So according to this analysis both check #1 and #2 are doing the same > thing and I don't see a way of check #1 getting passed and check #2 getting > failed. Please correct me if I am wrong. > > If this is correct shall we do the necessary adjustment to reduce the > complexity of the code? > > > [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/oauth2/authz/ > AuthorizationHandlerManager.java > [2] https://github.com/wso2-extensions/identity-inbound- > auth-oauth/blob/master/components/org.wso2.carbon. > identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/authz/ > AuthorizationHandlerManager.java#L100-L123 > [3] https://github.com/wso2-extensions/identity-inbound- > auth-oauth/blob/master/components/org.wso2.carbon. > identity.oauth/src/main/java/org/wso2/carbon/identity/ > oauth2/authz/handlers/AbstractResponseTypeHandler.java#L128-L165 > [4] https://github.com/wso2-extensions/identity-inbound- > auth-oauth/blob/master/components/org.wso2.carbon. > identity.oauth/src/main/java/org/wso2/carbon/identity/ > oauth2/authz/handlers/AbstractResponseTypeHandler.java#L66-L104 > > Thanks & Regards > Danushka Fernando > Associate Tech Lead > WSO2 inc. http://wso2.com/ > Mobile : +94716332729 <+94%2071%20633%202729> >
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
