1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-30 18:52:42 +01:00
phorge-phorge/src/applications/oauthserver/PhabricatorOAuthServerScope.php

22 lines
392 B
PHP
Raw Normal View History

OAuth Server enhancements -- more complete access token response and groundwork for scope Summary: this patch makes the access token response "complete" relative to spec by returning when it expires AND that the token_type is in fact 'Bearer'. This patch also lays the groundwork for scope by fixing the underlying data model and adding the first scope checks for "offline_access" relative to expires and the "whoami" method. Further, conduit is augmented to open up individual methods for access via OAuth generally to enable "whoami" access. There's also a tidy little scope class to keep track of all the various scopes we plan to have as well as strings for display (T849 - work undone) Somewhat of a hack but Conduit methods by default have SCOPE_NOT_ACCESSIBLE. We then don't even bother with the OAuth stuff within conduit if we're not supposed to be accessing the method via Conduit. Felt relatively clean to me in terms of additional code complexity, etc. Next up ends up being T848 (scope in OAuth) and T849 (let user's authorize clients for specific scopes which kinds of needs T850). There's also a bunch of work that needs to be done to return the appropriate, well-formatted error codes. All in due time...! Test Plan: verified that an access_token with no scope doesn't let me see anything anymore. :( verified that access_tokens made awhile ago expire. :( Reviewers: epriestley Reviewed By: epriestley CC: aran, epriestley Maniphest Tasks: T888, T848 Differential Revision: https://secure.phabricator.com/D1657
2012-02-21 23:28:05 +01:00
<?php
final class PhabricatorOAuthServerScope extends Phobject {
OAuth Server enhancements -- more complete access token response and groundwork for scope Summary: this patch makes the access token response "complete" relative to spec by returning when it expires AND that the token_type is in fact 'Bearer'. This patch also lays the groundwork for scope by fixing the underlying data model and adding the first scope checks for "offline_access" relative to expires and the "whoami" method. Further, conduit is augmented to open up individual methods for access via OAuth generally to enable "whoami" access. There's also a tidy little scope class to keep track of all the various scopes we plan to have as well as strings for display (T849 - work undone) Somewhat of a hack but Conduit methods by default have SCOPE_NOT_ACCESSIBLE. We then don't even bother with the OAuth stuff within conduit if we're not supposed to be accessing the method via Conduit. Felt relatively clean to me in terms of additional code complexity, etc. Next up ends up being T848 (scope in OAuth) and T849 (let user's authorize clients for specific scopes which kinds of needs T850). There's also a bunch of work that needs to be done to return the appropriate, well-formatted error codes. All in due time...! Test Plan: verified that an access_token with no scope doesn't let me see anything anymore. :( verified that access_tokens made awhile ago expire. :( Reviewers: epriestley Reviewed By: epriestley CC: aran, epriestley Maniphest Tasks: T888, T848 Differential Revision: https://secure.phabricator.com/D1657
2012-02-21 23:28:05 +01:00
Make OAuth scope handling more flexible 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. Reviewers: chad Reviewed By: chad Maniphest Tasks: T7303 Differential Revision: https://secure.phabricator.com/D15621
2016-04-05 19:48:00 +02:00
public static function getScopeMap() {
return array();
}
Make OAuth scope handling more flexible 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. Reviewers: chad Reviewed By: chad Maniphest Tasks: T7303 Differential Revision: https://secure.phabricator.com/D15621
2016-04-05 19:48:00 +02:00
public static function filterScope(array $scope) {
$valid_scopes = self::getScopeMap();
Make OAuth scope handling more flexible 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. Reviewers: chad Reviewed By: chad Maniphest Tasks: T7303 Differential Revision: https://secure.phabricator.com/D15621
2016-04-05 19:48:00 +02:00
foreach ($scope as $key => $scope_item) {
if (!isset($valid_scopes[$scope_item])) {
unset($scope[$key]);
}
}
OAuthServer polish and random sauce Summary: This diff makes the OAuthServer more compliant with the spec by - making it return well-formatted error codes with error types from the spec. - making it respect the "state" variable, which is a transparent variable the client passes and the server passes back - making it be super, duper compliant with respect to redirect uris -- if specified in authorization step, check if its valid relative to the client registered URI and if so save it -- if specified in authorization step, check if its been specified in the access step and error if it doesn't match or doesn't exist -- note we don't make any use of it in the access step which seems strange but hey, that's what the spec says! This diff makes the OAuthServer suck less by - making the "cancel" button do something in the user authorization flow - making the client list view and client edit view be a bit more usable around client secrets - fixing a few bugs I managed to introduce along the way Test Plan: - create a test phabricator client, updated my conf, and then linked and unlinked phabricator to itself - wrote some tests for PhabricatorOAuthServer -- they pass! -- these validate the various validate URI checks - tried a few important authorization calls -- http://phabricator.dev/oauthserver/auth/?client_id=X&state=test&redirect_uri=http://www.evil.com --- verified error'd from mismatching redirect uri's --- verified state parameter in response --- verified did not redirect to client redirect uri -- http://phabricator.dev/oauthserver/auth/?client_id=X w/ existing authorization --- got redirected to proper client url with error that response_type not specified -- http://phabricator.dev/oauthserver/auth/?client_id=X&response_type=code w/ existing authorization --- got redirected to proper client url with pertinent code! - tried a few important access calls -- verified appropriate errors if missing any required parameters -- verified good access code with appropriate other variables resulted in an access token - verified that if redirect_uri set correctly in authorization required for access and errors if differs at all / only succeeds if exactly the same Reviewers: epriestley Reviewed By: epriestley CC: aran, epriestley, ajtrichards Maniphest Tasks: T889, T906, T897 Differential Revision: https://secure.phabricator.com/D1727
2012-03-01 23:46:18 +01:00
Make OAuth scope handling more flexible 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. Reviewers: chad Reviewed By: chad Maniphest Tasks: T7303 Differential Revision: https://secure.phabricator.com/D15621
2016-04-05 19:48:00 +02:00
return $scope;
OAuthServer polish and random sauce Summary: This diff makes the OAuthServer more compliant with the spec by - making it return well-formatted error codes with error types from the spec. - making it respect the "state" variable, which is a transparent variable the client passes and the server passes back - making it be super, duper compliant with respect to redirect uris -- if specified in authorization step, check if its valid relative to the client registered URI and if so save it -- if specified in authorization step, check if its been specified in the access step and error if it doesn't match or doesn't exist -- note we don't make any use of it in the access step which seems strange but hey, that's what the spec says! This diff makes the OAuthServer suck less by - making the "cancel" button do something in the user authorization flow - making the client list view and client edit view be a bit more usable around client secrets - fixing a few bugs I managed to introduce along the way Test Plan: - create a test phabricator client, updated my conf, and then linked and unlinked phabricator to itself - wrote some tests for PhabricatorOAuthServer -- they pass! -- these validate the various validate URI checks - tried a few important authorization calls -- http://phabricator.dev/oauthserver/auth/?client_id=X&state=test&redirect_uri=http://www.evil.com --- verified error'd from mismatching redirect uri's --- verified state parameter in response --- verified did not redirect to client redirect uri -- http://phabricator.dev/oauthserver/auth/?client_id=X w/ existing authorization --- got redirected to proper client url with error that response_type not specified -- http://phabricator.dev/oauthserver/auth/?client_id=X&response_type=code w/ existing authorization --- got redirected to proper client url with pertinent code! - tried a few important access calls -- verified appropriate errors if missing any required parameters -- verified good access code with appropriate other variables resulted in an access token - verified that if redirect_uri set correctly in authorization required for access and errors if differs at all / only succeeds if exactly the same Reviewers: epriestley Reviewed By: epriestley CC: aran, epriestley, ajtrichards Maniphest Tasks: T889, T906, T897 Differential Revision: https://secure.phabricator.com/D1727
2012-03-01 23:46:18 +01:00
}
OAuth Server enhancements -- more complete access token response and groundwork for scope Summary: this patch makes the access token response "complete" relative to spec by returning when it expires AND that the token_type is in fact 'Bearer'. This patch also lays the groundwork for scope by fixing the underlying data model and adding the first scope checks for "offline_access" relative to expires and the "whoami" method. Further, conduit is augmented to open up individual methods for access via OAuth generally to enable "whoami" access. There's also a tidy little scope class to keep track of all the various scopes we plan to have as well as strings for display (T849 - work undone) Somewhat of a hack but Conduit methods by default have SCOPE_NOT_ACCESSIBLE. We then don't even bother with the OAuth stuff within conduit if we're not supposed to be accessing the method via Conduit. Felt relatively clean to me in terms of additional code complexity, etc. Next up ends up being T848 (scope in OAuth) and T849 (let user's authorize clients for specific scopes which kinds of needs T850). There's also a bunch of work that needs to be done to return the appropriate, well-formatted error codes. All in due time...! Test Plan: verified that an access_token with no scope doesn't let me see anything anymore. :( verified that access_tokens made awhile ago expire. :( Reviewers: epriestley Reviewed By: epriestley CC: aran, epriestley Maniphest Tasks: T888, T848 Differential Revision: https://secure.phabricator.com/D1657
2012-02-21 23:28:05 +01:00
}