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 Mon, Aug 21, 2017 at 9:35 AM, Hasanthi Purnima Dissanayake < [email protected]> wrote: > Hi Farasath, > > The logic behind returning claims in 'oidc' is based on the intersection > of both sp requested claims and the registry defined claims for the scope. > So in order to return a specific claims it should define in the registry > and it should define as a requested claims. > > 2. Improve the fix[2] to return all claims for *openid *flow only when >> service provider has no requested claims. >> >> So why do we need to return all the claims for openid flow when the SP > has no requested claims? > I think there is a small confusing here. In the our current implementation we are returing all the claims for *openid *(not *openidconnect*) request type[1]. So I wanted to improve the handler logic while maintaining backward compatibility for *openid* relying parties. [1] https://github.com/wso2/carbon-identity-framework/blob/77b83f1d2ca172366a86476395da3062c56c4fd6/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/handler/claims/impl/DefaultClaimHandler.java#L422 > > Thanks, > > Hasanthi Dissanayake > > Software Engineer | WSO2 > > E: [email protected] > M :0718407133| http://wso2.com <http://wso2.com/> > > On Mon, Aug 21, 2017 at 9:11 AM, Rushmin Fernando <[email protected]> > wrote: > >> yes, we should get rid of unwanted processing. >> >> IMO we should honour the configured requested claims in the service >> provider. But I'm not aware whether there was a need to send all the claims >> for open id. >> >> >> >> On Sat, Aug 19, 2017 at 7:48 PM, Farasath Ahamed <[email protected]> >> wrote: >> >>> Hi All, >>> >>> In the current implementation of the DefaultClaimHandler[1] claim >>> handling logic involves the below steps when retrieving claims for local >>> and federated scenarios, >>> >>> 1. Loading local claims and claims mappings >>> 2. Loading all non-empty claims of the user >>> >>> #1 involves several DB calls where as step #2 results in a call to the >>> user store which means either a DB call or LDAP/AD call depending on the >>> user store configured. >>> >>> Here are few shortcoming I noticed, >>> >>> 1. If a service provider has configured no requested claims, we >>> simply return an empty map of claims after going through the whole >>> process >>> #1 and #2. >>> 2. For authentication involved with flows like OAuth which do not >>> involve claims going through this claims handling logic doesn't make any >>> sense. >>> >>> >>> To give an idea of the performance impact, An authentication request >>> coming into the Authentication Framework takes about 950ms to complete. Of >>> this around 550ms is spent on handling claims (that's close to ~60%). So >>> for an OAuth flow with authorization code or implicit flow, this is a >>> performance hit. >>> >>> I initially did a fix for this[2], by returning an empty map of claims >>> if the there were no requested claims. But this doesn't seem to work since >>> we seem to return all available claims for *openid *flow[3]. >>> >>> Do we have a specific reason for return all available claims in the >>> openid flow? Shouldn't we honour service provider requested claims when >>> sending out user claims out of the framework? >>> >>> >>> I have a few improvements in my mind to overcome the problem, >>> >>> 1. Specifically, check for the *oauth *request type and stop executing >>> claim handling logic. >>> 2. Improve the fix[2] to return all claims for *openid *flow only when >>> service provider has no requested claims. >>> >>> Do you see any complexities that could arise with the suggested >>> improvements? >>> >>> >>> [1] https://github.com/wso2/carbon-identity-framework/blob/m >>> aster/components/authentication-framework/org.wso2.carbon.id >>> entity.application.authentication.framework/src/main/java/or >>> g/wso2/carbon/identity/application/authentication/framework/ >>> handler/claims/impl/DefaultClaimHandler.java >>> >>> [2] https://github.com/wso2/carbon-identity-framework/pull/961 >>> >>> [3] https://github.com/wso2/carbon-identity-framework/blob/m >>> aster/components/authentication-framework/org.wso2.carbon.id >>> entity.application.authentication.framework/src/main/java/or >>> g/wso2/carbon/identity/application/authentication/framework/ >>> handler/claims/impl/DefaultClaimHandler.java#L422 >>> >>> >>> Thanks, >>> 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> >>> >>> >>> >> >> >> -- >> *Best Regards* >> >> *Rushmin Fernando* >> *Technical Lead* >> >> WSO2 Inc. <http://wso2.com/> - Lean . Enterprise . Middleware >> >> mobile : +94775615183 >> >> >> >> _______________________________________________ >> Dev mailing list >> [email protected] >> http://wso2.org/cgi-bin/mailman/listinfo/dev >> >> >
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
