-
Notifications
You must be signed in to change notification settings - Fork 193
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
feat: rewrite Optimize API auth guide #4315
Conversation
👋 🤖 ✅ Looks like the changes were ported across versions, nice job! 🎉 You can read more about the versioning within our docs in our documentation guidelines. |
|
||
<TabItem value='self-managed'> | ||
|
||
1. [Configure the `api.audience` setting](/self-managed/optimize-deployment/configuration/system-configuration.md#public-api) in your Optimize installation to match the audience property of the **Optimize API** [API in Identity]($docs$/self-managed/identity/user-guide/additional-features/adding-an-api/). |
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 is different from other API Auth guides, and I don't feel great about requesting that someone re-configure their environment in order to get an auth token....but as discussed in Slack, the out-of-the-box configuration of a Self-Managed instance doesn't actually work for client_credentials authentication.
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.
Lgtm, just some suggestions for formatting and styling 👍
|
||
1. [Create client credentials]($docs$/guides/setup-client-connection-credentials/) in the **Clusters > Cluster name > API** tab of [Camunda Console](https://console.camunda.io/). | ||
2. Add permissions to this client for **Optimize**. | ||
3. Upon creating the client, capture the following values required to generate a token: |
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.
3. Upon creating the client, capture the following values required to generate a token: | |
3. After creating the client, capture the following values required to generate a token: |
Suggestion, "upon" doesn't do well in translation here (future proofing).
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.
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.
Resolution from 4138: it should read "Once you have created the client, capture...".
I'll do this in my follow-up.
|
||
All Optimize API requests except [the health readiness endpoint](./health-readiness.md) require authentication. To authenticate, generate a [JSON Web Token (JWT)](https://jwt.io/introduction/) and include it in each request. | ||
|
||
## Generating a token |
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.
## Generating a token | |
## Generate a token |
Suggestion to get away from using gerunds and make it more active, I think I suggested this in other reviews so not sure whether this was adopted - ignore if not, to keep consistent.
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.
Ignoring for now for consistency; I'd like to get through all these rewrites (only one more after this!), and we can standardize this later.
| Authorization Server URL | `ZEEBE_AUTHORIZATION_SERVER_URL` | `https://login.cloud.camunda.io/oauth/token` | | ||
| Optimize REST Address | `CAMUNDA_OPTIMIZE_BASE_URL` | - | | ||
<!-- this comment convinces the markdown processor to still treat the table as a table, but without adding surrounding paragraphs. 🤷 --> | ||
:::tip |
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.
:::tip | |
:::caution |
Seeing as this is a crucial step if they don't capture the secret, should we make the admonition a caution?
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.
No preference; I will adjust across all auth guides in a follow-up.
:::tip | ||
When client credentials are created, the `Client Secret` is only shown once. Save this `Client Secret` somewhere safe. | ||
::: | ||
4. Execute an authentication request to the token issuer: |
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.
Not for this PR, but I notice we use "execute" a lot, and I'm not sure if we should be replacing this term with "run" or something similar, perhaps not here but wanted to capture this somewhere for discussion.
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.
No preference; let me know of any specific changes you'd like me to make in a follow-up.
--data-urlencode "client_id=${ZEEBE_CLIENT_ID}" \ | ||
--data-urlencode "client_secret=${ZEEBE_CLIENT_SECRET}" | ||
``` | ||
5. A successful authentication response looks like the following: |
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.
Not sure if this should be a step, as technically each numbered bullet should be an action, so perhaps this belongs in the next step, or as a sub paragraph or something in the previous step? Without seeing the actual doc, I can't envisage what this looks like, so it might not be better when actually rendered, but jsut something I wanted to note.
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.
|
||
<TabItem value='self-managed'> | ||
|
||
1. [Configure the `api.audience` setting](/self-managed/optimize-deployment/configuration/system-configuration.md#public-api) in your Optimize installation to match the audience property of the **Optimize API** [API in Identity]($docs$/self-managed/identity/user-guide/additional-features/adding-an-api/). |
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.
Should "audience" property here be in code (not sure, just asking)?
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 don't know either, but if someone does and gives me specific changes, I'll take care of them in my follow-up.
7. Capture the value of the `access_token` property and store it as your token. | ||
|
||
:::note | ||
The Optimize API can also be configured in a Self-Managed environment to authenticate with a single shared access token. Refer to [Public API Configuration](/self-managed/optimize-deployment/configuration/system-configuration.md#public-api) for the configuration required to access the public API using a specific token. |
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 Optimize API can also be configured in a Self-Managed environment to authenticate with a single shared access token. Refer to [Public API Configuration](/self-managed/optimize-deployment/configuration/system-configuration.md#public-api) for the configuration required to access the public API using a specific token. | |
The Optimize API can also be configured in a Self-Managed environment to authenticate using a single shared access token. See [Public API Configuration](/self-managed/optimize-deployment/configuration/system-configuration.md#public-api) for the configuration required to access the public API using a specific token. |
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'll adjust this across all auth guides in a follow-up.
import Tabs from "@theme/Tabs"; | ||
import TabItem from "@theme/TabItem"; | ||
|
||
All Optimize API requests except [the health readiness endpoint](./health-readiness.md) require authentication. To authenticate, generate a [JSON Web Token (JWT)](https://jwt.io/introduction/) and include it in each request. |
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.
All Optimize API requests except [the health readiness endpoint](./health-readiness.md) require authentication. To authenticate, generate a [JSON Web Token (JWT)](https://jwt.io/introduction/) and include it in each request. | |
All Optimize API requests except the [health readiness](./health-readiness.md) endpoint require authentication. To authenticate, generate a [JSON Web Token (JWT)](https://jwt.io/introduction/) and include it in each request. |
Suggestion to just have the endpoint name in the link.
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.
No preference on my end, I'll adjust across all APIs in a follow-up.
|
||
Include the previously captured token as an authorization header in each request: `Authorization: Bearer <TOKEN>`. | ||
|
||
For example, to send a request to the Optimize API's ["Get dashboard IDs" endpoint](./dashboard/get-dashboard-ids.md): |
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.
For example, to send a request to the Optimize API's ["Get dashboard IDs" endpoint](./dashboard/get-dashboard-ids.md): | |
For example, to send a request to the Optimize API's ["Get dashboard IDs"](./dashboard/get-dashboard-ids.md) endpoint: |
Also, should this endpoint name be in quotes?
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 don't know. I can make these changes in a follow-up, if we decide they're needed.
@mesellings Thanks for the review! I'm going to temporarily ignore your suggestions for this PR, and leave all conversations unresolved, but add an item to #4117 to apply them to all the API auth guides. And then I'll take care of them after I finish this PR + one for the Zeebe API. |
@RomanJRW can you or someone from your team please review this PR, to confirm that I have not introduced incorrect information or steps for Optimize API authentication? |
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.
🏃
🧹 Preview environment for this PR has been torn down. |
Description
Part of #4117.
Rewrites the Optimize API authentication guide according to specs defined in #4117.
When should this change go live?
hold
label or convert to draft PR)PR Checklist
/versioned_docs
directory./docs
directory (aka/next/
).