Skip to content

Authorization improvements#3

Open
jamesmehorter wants to merge 7 commits into
bbaisley:masterfrom
penske-media-corp:feature/authorization_improvements
Open

Authorization improvements#3
jamesmehorter wants to merge 7 commits into
bbaisley:masterfrom
penske-media-corp:feature/authorization_improvements

Conversation

@jamesmehorter
Copy link
Copy Markdown

See commit messages for detail on each adjustment.

This PR accomplishes:

  • Adding a password authentication method
  • Abstracting existing functionality to support multiple authentication methods
  • Ensures that each authentication method (auth code, password, and token refresh) all update internal variables and the set appropriate Authentication header for Presto.

Changes to setAccess():
	+ setAccess() previously updated internal vars with those passed into the method.
	+ The setting of internal class vars is now handled within each token fetching method. This removes the need for the setAccess method, and rightly so—the token fetching methods should handle this task (or possibly abstract it further later on).
	+ setAccess() previously set the authentication header for the rest client.
	+ The Authorization header for the rest client is now handled within it's own 'setAccessHeaders' method, and is called in each token fetching method. This was done with the same justification as the previous item.

Changes to checkAccess():
	+ checkAccess() previously returned bool, but also refreshed the access token. That refresh has been abstracted out. It can now be called like so in usage code:

	if ( $api->isAccessTokenExpired() ) {
		$api->refreshAccess();
	}

	+ this allows developers to cache the $api object and refresh their token only when needed.
Now that we have a method to authenticate via password as well let's rename this method so both intuitively named
Again, the github diff shows odd whitespace—Hmm
If the token expiration has already occurred it's 'expired'.
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