Skip to content

Added scope validation per endpoint#60

Open
capnslipp wants to merge 2 commits intodaneden:mainfrom
capnslipp:main
Open

Added scope validation per endpoint#60
capnslipp wants to merge 2 commits intodaneden:mainfrom
capnslipp:main

Conversation

@capnslipp
Copy link

@capnslipp capnslipp commented Nov 21, 2022

Added scope validation per endpoint, based on the “OAuth 2.0 scopes required by this endpoint” section listed for each endpoint in the Twitter API v2 docs (https://developer.twitter.com/en/docs/twitter-api).

Motivation for this addition is that if you're not using a scope of Set(OAuth2Scope.allCases) when authentication with OAuth2, it can be easy to call an endpoint you don't have the proper scope for and only get back a TwitterAPIError with a non-specific HTTP 401 Unauthorized message (same error you'd get if you weren't properly authenticated), which can be confusing for 1st-time users of Twift setting things up in their app. Further, since the call to Twift.Authentication().authenticateUser(…, scope: …, …) and the call to a twiftClient.get…() endpoint often live in different files in app code, it's not obvious where the issue is.

This addition checks the OAuth2User's scopes against the required scopes for the Twitter endpoint (APIRoute + HTTPMethod), via new internal requiredScopes(for:APIRoute,method:HTTPMethod) -> Set<OAuth2Scope> helper method. The result (in a failure state) is that the client app will get a (new) specific TwiftError.UnauthorizedForRequiredScopes(_:,missingScopes:) exception instead of a generic HTTP 401 TwitterAPIError without actually making the invalid call to the Twitter API. I also considered doing this check in decodeOrThrow<…>(…) (where TwitterAPIError is thrown from) only after we get a 401 back from the Twitter API, but decided that a more immediate exception and avoiding hitting Twitter with invalid requests is probably better overall.

  • At the beginning of call<>(route:,method:,queryItems:,body:), now checking if the requiredScopes for this endpoint (route + method) is a subset of the current oauthUser.scope. If not, throwing a TwiftError.UnauthorizedForRequiredScopes.
  • Created new internal util method requiredScopes(for route: APIRoute, method: HTTPMethod) which returns a Set<OAuth2Scope> containing all the required scopes for the endpoint.
    ‣ Uses calls to new HTTP-method-specific requiredScopesFor…Route() util methods to produce the return value.
  • Created new fileprivate HTTP-method-specific helper methods requiredScopesForGET/POST/PUT/DELETERoute(_ route: APIRoute) which switch on the route returning the API doc-specified Set<OAuth2Scope> list for all valid routes, and defaulting to emitting a fatalError(…) if the route is not valid for that method.
    ‣ Decided to fatalError(…) instead of throwing a Swift error because a missing route would be an issue in Twift's code, not app client code— so an error we're guaranteed to see in Twift's Tests.
  • Added new TwiftError.UnauthorizedForRequiredScopes(_ requiredScopes: Set<OAuth2Scope>, missingScopes: Set<OAuth2Scope>) error.
  • Added doc comments to new requiredScopes…(…) methods and call<…>(…) method

Added scope validation per endpoint, based on the “OAuth 2.0 scopes required by this endpoint” section listed for each endpoint in the Twitter API v2 docs (https://developer.twitter.com/en/docs/twitter-api).

Motivation for this addition is that if you're not using a scope of `Set(OAuth2Scope.allCases)` when authentication with OAuth2, it can be easy to call an endpoint you don't have the proper scope for and only get back a `TwitterAPIError` with a non-specific HTTP 401 Unauthorized message (same error you'd get if you weren't properly authenticated), which can be confusing for 1st-time users of Twift setting things up in their app.  Further, since the call to `Twift.Authentication().authenticateUser(…, scope: …, …)` and the call to a `twiftClient.get…()` endpoint often live in different files in app code, it's not obvious where the issue is.

This addition checks the `OAuth2User`'s scopes against the required scopes for the Twitter endpoint (`APIRoute` + `HTTPMethod`), via new internal `requiredScopes(for:APIRoute,method:HTTPMethod) -> Set<OAuth2Scope>` helper method.  The result (in a failure state) is that the client app will get a (new) specific `TwiftError.UnauthorizedForRequiredScopes(_:,missingScopes:)` exception instead of a generic HTTP 401 `TwitterAPIError` _without actually making the invalid call to the Twitter API._  I also considered doing this check in `decodeOrThrow<…>(…)` (where `TwitterAPIError` is thrown from) only after we get a 401 back from the Twitter API, but decided that a more immediate exception and avoiding hitting Twitter with invalid requests is probably better overall.

* At the beginning of `call<>(route:,method:,queryItems:,body:)`, now checking if the `requiredScopes` for this endpoint (`route` + `method`) is a subset of the current `oauthUser.scope`.  If not, throwing a `TwiftError.UnauthorizedForRequiredScopes`. 
* Created new internal util method `requiredScopes(for route: APIRoute, method: HTTPMethod)` which returns a `Set<OAuth2Scope>` containing all the required scopes for the endpoint.  
	‣ Uses calls to HTTP-method-specific `requiredScopesFor…Route()` methods to produce the return value.
* Created new fileprivate HTTP-method-specific helper methods `requiredScopesForGET`/`POST`/`PUT`/`DELETERoute(_ route: APIRoute)` which switch on the `route` returning the API doc-specified `Set<OAuth2Scope>` list for all valid routes, and defaulting to emitting a `fatalError(…)` if the `route` is not valid for that method.  
	‣ Decided to `fatalError(…)` instead of throwing a Swift error because a missing route would be an issue in Twift's code, not app client code— so an error we're guaranteed to see in Twift's Tests.
* Added new `TwiftError.UnauthorizedForRequiredScopes(_ requiredScopes: Set<OAuth2Scope>, missingScopes: Set<OAuth2Scope>)` error.
@capnslipp
Copy link
Author

FYI: I've run unit test for Twift target and everything passes, and built & run Twift_SwiftUI and everything seems to still work there too.

@capnslipp
Copy link
Author

All GH Action tests passed in my fork: https://github.com/capnslipp/Twift/actions/runs/3511241121

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant