Page MenuHomePhabricator

Make OAuth scope handling more flexible
ClosedPublic

Authored by epriestley on Apr 5 2016, 6:11 PM.

Details

Summary

Ref T7303. Currently, our handling of "scope" is fairly rigid and adheres to the spec, but some of these behaviors don't make much sense in practice.

Soften some behaviors and make them more flexible:

Soft Failure on Unknown Permissions: If a client asks for a permission we don't know about, just warn that we don't recognize it instead of fataling. In particular, I plan to make offline_access and whoami implicit. Older clients that request these permissions will still work fine as long as we don't hard-fatal.

Move user.whoami to ALWAYS scope: Make whoami a default permission. We've already done this, in effect; this just formalizes it.

Tokens no longer expire: Make offline_access (infinite-duration tokens) a default permission. I think the OAuth model doesn't map well to reality. It is common for other providers to issue "temporary" tokens with a duration of multiple years, and the refesh workflow is sort of silly. We can add a "temporary" scope later if we need temporary tokens.

This flow was potentially extra silly with the "log out of Phacility" use case, where we might need to have you log in again before we could log you out, which is bizarre and senseless. Avoid this nonsense.

Move away from granular permissions: Users currently get to pick-and-choose which permissions they grant, but this likely rarely/never works in practice and is fairly hostile since applications can't communicate which permissions they need. Applications which can actually operate with only some subset of permissions can make separate requests (e.g., when you activate "cool feature X", it asks for X permission). I think applications that do this are rare; pretty much everything just asks for tons of permissions and everyone grants them.

Making this all-or-nothing is better for well-behaved applications and better for users. It's also slightly better for overzealous applications that ask for more than they need, but whatever. Users can make an informed decision, hopefully, and I plan to let administrators force applications to a subset of permissions once we introduce meaningful scopes.

Test Plan
  • Generated tokens.
  • Used tokens.
  • Authorized an instance.
  • Faked some bogus scopes, got clean authorization.

Diff Detail

Repository
rP Phabricator
Branch
oauth7
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 11511
Build 14367: Run Core Tests
Build 14366: arc lint + arc unit

Event Timeline

epriestley retitled this revision from to Make OAuth scope handling more flexible.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
chad accepted this revision.Apr 5 2016, 8:45 PM
chad edited edge metadata.
This revision is now accepted and ready to land.Apr 5 2016, 8:45 PM
This revision was automatically updated to reflect the committed changes.