kratos icon indicating copy to clipboard operation
kratos copied to clipboard

fix: check return code of ms graphapi /me request.

Open floriankramer opened this issue 2 years ago • 2 comments

When using the microsoft oicd provider and tyring to link an account with insufficient scope (e.g. missing User.Read) for the authorization code the request to https://graph.microsoft.com/v1.0/me used to determine the user id fails silently. This request is used when building the Context to get the user id, which is used as a unique identifier for the credentials.

The result is that the identifier in the identity_crendential_identifiers table is microsoft:. As soon as a second user tries to link their account ory attempts to create a secondcredential identifier with the same identifier of microsoft:. The linking then fails, and the user logs in as the first user who linked with microsoft if they use microsoft oicd.

This pr adds a check to make sure the status code returned from the graph api is 200. There will be a separate pr from a colleague that updates the documentation to make it more clear which scopes are needed for microsoft oicd.

Related issue(s)

Checklist

  • [x] I have read the contributing guidelines.
  • [x] I have referenced an issue containing the design document if my change introduces a new feature.
  • [x] I am following the contributing code guidelines.
  • [x] I have read the security policy.
  • [x] I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security. vulnerability, I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.
  • [ ] I have added tests that prove my fix is effective or that my feature works.
  • [ ] I have added or changed the documentation.

Further Comments

floriankramer avatar Aug 05 '22 14:08 floriankramer

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Aug 05 '22 14:08 CLAassistant

I haven't touched anything that should trigger the linter errors (and am getting very different errors locally in code that I've also not changed), so it seems like they are caused by upstream code.

floriankramer avatar Aug 08 '22 09:08 floriankramer

The linter errors came from a premature update of the gofmt binary and our use of Open API. I updated the PR, with our state in master. Should be fixed now. :)

jonas-jonas avatar Sep 09 '22 09:09 jonas-jonas

Hello @floriankramer Congrats on merging your first PR in Ory 🎉 ! Your contribution will soon be helping secure millions of identities around the globe 🌏. As a small token of appreciation we send all our first time contributors a gift package to welcome them to the community. Please drop me an email and I will forward you the form to claim your Ory swag!

vinckr avatar Sep 14 '22 14:09 vinckr