-
Notifications
You must be signed in to change notification settings - Fork 49
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
[WIP] Add Nextdoor Keyring Service #113
base: trunk
Are you sure you want to change the base?
Conversation
|
||
function basic_ui_intro() { | ||
/* translators: url */ | ||
echo '<p>' . sprintf( __( 'To use the Nextdoor service you need to be manually approved. Please reach out to get your client id and client secret.', 'keyring' )) . '</p>'; |
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.
The end user doesn't need to provide client id and secret, which is stored in the WordPress server.
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.
Anybody using this service will need a client ID and secret. We should probably provide a link to a support doc or something where they can request credentials.
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.
We don't have a full on support doc yet and I'm not sure if we will be creating one soon. Do you think leaving an email to contact would be fine?
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.
} | ||
|
||
function build_token_meta( $token ) { | ||
$decoded_id_token = json_decode(base64_decode(explode('.', $token['id_token'])[1])); |
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 looks pretty risky, but I think it's only like this while this is in development. We should probably put some safeguards around this.
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.
Yeah, I'm using explode
to parse the id_token
since we only wanted to grab the payload and not the header
and signature
of the token.
this looks promising
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.
Sure! It might just be a good idea to check that we have id_token
and at least 2 elements after the explode
etc. Although we might be able to guarantee all that before we get here.
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.
Looks like that stack overflow post doesn't work for our case. I added some safeguards for now.
Let's look into this during our meeting, it shouldn't be an issue with using a UK account right @renjunl ? |
Hi @pablinos , I addressed the one comment you made and added some safeguards. I looked into it a bit more and it looks like even after adding a name and an external ID, duplicate connections keep being made for nextdoor. |
We are currently in the process of adding Nextdoor as an option to Wordpress's auto-social sharing feature. In order for this work to be completed, the first step is adding Nextdoor as a Keyring service in order to handle authentication / storing tokens.