We have moved to GitHub Issues
Created by Shannon Deminick 27 Feb 2018, 07:25:53 Updated by Sebastiaan Janssen 28 Jun 2018, 09:28:46
Relates to: U4-11486
There's a public ticket here: https://github.com/umbraco/UmbracoRestApi/issues/42 which describes the issue and is affecting authenticating via the rest api.
This is a regression issue with changes to auth made in 7.8 where we properly track the session id in the db now.
Issue is: starting with 7.8, the rest API cannot authenticate. It is possible to get a token, but that token is rejected (401).
Tracing UmbracoAuthorizeAttribute leads to UmbracoBackOfficeIdentity.FromClaimsIdentity() which ensures that all claims defined by RequiredBackOfficeIdentityClaimTypes are present. Starting with 7.8, this list now contains SecurityStamp, which is missing.
That claim is added in UmbracoBackOfficeIdentity.AddUserDataClaims() but only when the SessionId claim is not present. However, when the identity is created by BackOfficeClaimsIdentityFactory.CreateAsync, the initial call to base.CreateAsync() returns an identity that ''does'' contain the SessionId. That claim is copied by AddExistingClaims() and therefore prevents the SecurityStamp claim from being added by AddUserDataClaims().
Should potentially affect anything using token-based identity, not only WebAPI. I.e... kinda critical?
Fixed by making sure we always add the SecurityStamp claim: see PR https://github.com/umbraco/Umbraco-CMS/pull/2510.
Review and repro: see https://github.com/umbraco/UmbracoRestApi/issues/42 Should work with 7.7.13, fail with 7.8 and above, and work again with the PR. Tested OK with a basic RestAPI example.
Suggesting @Shandem has a look, as I quite don't know whether it is all OK, or hiding a more complex issue with identities.
Also, when merging, please update/close the issue on GitHub!
Perfect. The reason why the SecurityStamp wasn't set unless the sessionid was also null is because previously the sessionid was the security stamp and now it is not. So we should have discovered this issue already but i guess we are lucky that we haven't.
Tested and all working, nice work on figuring this one out, must have been interesting ;)
Backwards Compatible: True
Affected versions: 7.8.0, 7.9.0, 7.8.1, 7.9.1, 7.9.2
Due in version: 7.9.3
Sprint: Sprint 80
Story Points: 1