-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add license key to auth button #85
Conversation
This prevented all disconnect functionality
@@ -266,7 +266,7 @@ public function it_should_render_correct_html_for_oauth_resource_with_license_ke | |||
update_option( 'test_storage', [ | |||
'stellarwp_auth_url_service_oauth_with_license_key_field_1' => [ | |||
'expiration' => 0, | |||
'value' => 'https://licensing.stellarwp.com/account-auth?uplink_domain=&uplink_slug=service-oauth-with-license-key-field-1&_uplink_nonce=535281edcd', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious why this is being removed? Was the test passing before? Did something change to actually not require those query args? Were they useless before and this is cleanup?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, they were unnecessary when the test was first added. Those values are combined and encoded for the uplink_callback
parameter here in Auth_Url_Builder::build()
:
uplink/src/Uplink/Auth/Auth_Url_Builder.php
Lines 59 to 80 in 11b5e6d
// Query arguments to combine with $_GET and add to the authorization URL. | |
$args = [ | |
'uplink_domain' => $domain, | |
'uplink_slug' => $slug, | |
]; | |
// Optionally include a license key if set. | |
if ( ! empty( $this->license_key ) ) { | |
$args['uplink_license'] = $this->license_key; | |
} | |
$url = add_query_arg( | |
array_filter( array_merge( $_GET, $args ) ), | |
admin_url( $pagenow ) | |
); | |
return sprintf( '%s?%s', | |
$auth_url, | |
http_build_query( [ | |
'uplink_callback' => base64_encode( $this->nonce->create_url( $url ) ), | |
] ) | |
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to update this because of the new data attribute, so I made some changes there too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks fantastic! 👯
When a site needs to connect to the licensing server to generate a token or connect securely with an external service, a license key should be passed with the request. This attaches the license key to the auth button and adjusts a little of the functionality to make the UX more fluid for the user.
This also fixes some styling of the button so that it displays to the right of the license field that is used for authentication.
No License Key Present
License Key Invalid
License Key Valid
Site Connected