Skip to content
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

Readme: update example public key format #104

Merged
merged 1 commit into from Oct 26, 2023

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Oct 25, 2023

Fixes #103

The readme suggests setting up keys in wp-config.php, and suggests using comments like -----BEGIN RSA PUBLIC KEY----- to clearly mark the beginning and end of your key.

However, that format:

  1. Does not match the output of the openssl command.
  2. Does not match the format we expect in Tools > Site Health:
    0 === strpos( OIDC_PUBLIC_KEY, '-----BEGIN PUBLIC KEY-----' )

Let's update the example format to match what's expected.

Fixes #103

The readme suggests setting up keys in `wp-config.php`, and suggests using comments like `-----BEGIN RSA PUBLIC KEY-----` to clearly mark the beginning and end of your key.

However, that format:

1. Does not match the output of the `openssl` command.
2. Does not match the format we expect in Tools > Site Health: https://github.com/Automattic/wp-openid-connect-server/blob/ee663a12d9e12d0b5c9e8c7745c3b48c716754b2/src/SiteStatusTests.php#L35

Let's update the example format to match what's expected.
@jeherve jeherve added the bug Something isn't working label Oct 25, 2023
@jeherve jeherve self-assigned this Oct 25, 2023
@ashfame
Copy link
Member

ashfame commented Oct 26, 2023

@jeherve Thanks for your contribution! Indeed the output doesn't have RSA keyword in PEM headers when I tried running openssl on mac and on linux. So, I am going to merge this PR so that readme reflects it better and also following up with another PR to improve the key detection so that if someone has RSA keyword in PEM headers, it doesn't raise an alarm for that.

@ashfame ashfame self-requested a review October 26, 2023 11:59
@ashfame ashfame merged commit 3b7422b into main Oct 26, 2023
1 check passed
@ashfame ashfame deleted the edit/readme-public-key-format branch October 26, 2023 12:00
ashfame added a commit that referenced this pull request Oct 26, 2023
ashfame added a commit that referenced this pull request Nov 2, 2023
fix readme for private key example as done in #104
@ashfame ashfame mentioned this pull request Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Site health says oidc keys are invalid, but they aren't
2 participants