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

[DX-1766] Documented JWE config options #5681

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Conversation

sredxny
Copy link
Contributor

@sredxny sredxny commented Oct 30, 2024

User description

For internal users - Please add a Jira DX PR ticket to the subject!



Preview Link

Ticket: https://tyktech.atlassian.net/browse/DX-1766

Description


Screenshots (if appropriate)


Checklist

  • I have added a preview link to the PR description.
  • I have reviewed the suggestions made by our AI (PR Agent) and updated them accordingly (spelling errors, rephrasing, etc.)
  • I have reviewed the guidelines for contributing to this repository.
  • I have read the technical guidelines for contributing to this repository.
  • Make sure you have started your change off our latest master.
  • I labeled the PR

PR Type

Documentation


Description

  • Added documentation for JSON Web Encryption (JWE) configuration options in Tyk Identity Broker profiles.
  • Introduced a step-by-step guide for setting up JWE, including prerequisites, setup process, and verification steps.
  • Provided troubleshooting tips for common issues encountered during JWE setup.
  • Updated the menu title for TIB integration examples to reflect the new content.

Changes walkthrough 📝

Relevant files
Documentation
about-profiles.md
Document JWE Configuration Options in TIB Profiles             

tyk-docs/content/tyk-stack/tyk-identity-broker/about-profiles.md

  • Added documentation for JWE configuration options.
  • Introduced fields JWE.Enabled and JWE.PrivateKeyLocation.
  • +12/-10 
    auth-user-for-api-access-github-oauth.md
    Add JWE Setup Guide and Troubleshooting                                   

    tyk-docs/content/tyk-stack/tyk-identity-broker/auth-user-for-api-access-github-oauth.md

  • Changed the title to "Practical TIB Integration Examples".
  • Added a step-by-step guide for JWE setup.
  • Included troubleshooting section for JWE setup.
  • +56/-1   
    menu.yaml
    Update Menu Title for TIB Integration Examples                     

    tyk-docs/data/menu.yaml

    • Updated menu title to "Practical TIB Integration Examples".
    +1/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    github-actions bot commented Oct 30, 2024

    PR Reviewer Guide 🔍

    (Review updated until commit a140599)

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Consistency Issue
    The capitalization of 'yes' in the 'Required' column for 'UseProviders.Key' and 'UseProviders.Secret' is inconsistent with other entries. It should be capitalized as 'Yes' for uniformity.

    Documentation Clarity
    The troubleshooting section could benefit from more detailed explanations or additional examples to ensure clarity and comprehensiveness.

    Copy link
    Contributor

    github-actions bot commented Oct 30, 2024

    PR Code Suggestions ✨

    Latest suggestions up to a140599

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Clarify the necessity of the 'openid' scope for OpenID usage

    Clarify the description for "UseProviders.Scopes" to explicitly state that the
    'openid' scope is mandatory when using OpenID, as the current wording could be
    misinterpreted.

    tyk-docs/content/tyk-stack/tyk-identity-broker/about-profiles.md [85]

    -| UseProviders.Scopes              | Specifies the level of access or permissions a client is requesting from the user and the authorization server, for example ["openid","email"].                                                                                                   | No, however when using openID the scope ‘openid’ should be added |
    +| UseProviders.Scopes              | Specifies the level of access or permissions a client is requesting from the user and the authorization server, for example ["openid","email"].                                                                                                   | No, but 'openid' is mandatory when using OpenID                 |
    Suggestion importance[1-10]: 6

    Why: Clarifying the description helps prevent potential confusion about the requirements, making the documentation more user-friendly and accurate.

    6
    Provide a description for the new section to enhance reader comprehension

    Add a brief description under the new title "Practical TIB Integration Examples" to
    provide context and improve the reader's understanding of the content that follows.

    tyk-docs/content/tyk-stack/tyk-identity-broker/auth-user-for-api-access-github-oauth.md [3]

     Title: Practical TIB Integration Examples
    +Description: This section provides practical examples and detailed guides for integrating Tyk Identity Broker (TIB) with various identity providers.
    Suggestion importance[1-10]: 6

    Why: Adding a description provides context and improves understanding, enhancing the overall quality of the documentation.

    6
    Standardize capitalization in the "Required" column for consistency

    Correct the capitalization inconsistency in the "Required" column for the fields
    "UseProviders.Key" and "UseProviders.Secret" to maintain uniformity and avoid
    potential confusion.

    tyk-docs/content/tyk-stack/tyk-identity-broker/about-profiles.md [82-83]

    -| UseProviders.Key                 | Oauth Client key                                                                                                                                                                                                                                  | yes                                                              |
    -| UseProviders.Secret              | Oauth Client Secret                                                                                                                                                                                                                               | yes                                                              |
    +| UseProviders.Key                 | Oauth Client key                                                                                                                                                                                                                                  | Yes                                                              |
    +| UseProviders.Secret              | Oauth Client Secret                                                                                                                                                                                                                               | Yes                                                              |
    Suggestion importance[1-10]: 5

    Why: Correcting the capitalization enhances consistency and professionalism in the documentation, though it's a minor cosmetic change.

    5
    Typo
    Start the description with a capital letter for uniformity

    Ensure that the description for "UseProviders.DiscoverURL" starts with a capital
    letter to maintain consistency with other descriptions in the table.

    tyk-docs/content/tyk-stack/tyk-identity-broker/about-profiles.md [84]

    -| UseProviders.DiscoverURL         | used to dynamically retrieve the OpenID Provider's configuration metadata, including endpoints and supported features, in JSON format from /.well-known/openid-configuration.                                                                     | Only required when using openid-connect                          |
    +| UseProviders.DiscoverURL         | Used to dynamically retrieve the OpenID Provider's configuration metadata, including endpoints and supported features, in JSON format from /.well-known/openid-configuration.                                                                     | Only required when using openid-connect                          |
    Suggestion importance[1-10]: 5

    Why: Starting the description with a capital letter improves readability and maintains consistency across the document, which is beneficial but not critical.

    5

    Previous suggestions

    Suggestions up to commit d5d51c6
    CategorySuggestion                                                                                                                                    Score
    Typo
    Correct a typo in a field name to enhance documentation clarity

    Correct the typo in the field name "ExrtactUserNameFromBasicAuthHeader" to
    "ExtractUserNameFromBasicAuthHeader" to ensure clarity and correctness in
    documentation.

    tyk-docs/content/tyk-stack/tyk-identity-broker/about-profiles.md [74]

    -| ExrtactUserNameFromBasicAuthHeader | A boolean value that, when set to true, instructs TIB to gather the user and password from the Authorization header when handling the request. | No |
    +| ExtractUserNameFromBasicAuthHeader | A boolean value that, when set to true, instructs TIB to gather the user and password from the Authorization header when handling the request. | No |
    Suggestion importance[1-10]: 8

    Why: Correcting the typo from "ExrtactUserNameFromBasicAuthHeader" to "ExtractUserNameFromBasicAuthHeader" significantly improves the accuracy and professionalism of the documentation.

    8
    Enhancement
    Standardize capitalization in documentation for consistency

    Ensure consistency in capitalization for the "Required" column values for
    "UseProviders.Key" and "UseProviders.Secret" by changing "yes" to "Yes".

    tyk-docs/content/tyk-stack/tyk-identity-broker/about-profiles.md [82-83]

    -| UseProviders.Key                 | Oauth Client key                                                                                                                                                                                                                                  | yes                                                              |
    -| UseProviders.Secret              | Oauth Client Secret                                                                                                                                                                                                                               | yes                                                              |
    +| UseProviders.Key                 | Oauth Client key                                                                                                                                                                                                                                  | Yes                                                              |
    +| UseProviders.Secret              | Oauth Client Secret                                                                                                                                                                                                                               | Yes                                                              |
    Suggestion importance[1-10]: 5

    Why: Standardizing the capitalization of "yes" to "Yes" in the "Required" column enhances consistency and readability in the documentation, though it's a relatively minor improvement.

    5
    Suggestions up to commit 438a829
    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Standardize capitalization in table entries for consistency

    Standardize the capitalization of the 'Required' field values for consistency.
    Change 'yes' to 'Yes' in the 'UseProviders.Key' and 'UseProviders.Secret' rows.

    tyk-docs/content/tyk-stack/tyk-identity-broker/about-profiles.md [82-83]

    -| UseProviders.Key                 | Oauth Client key                                                                                                                                                                                                                                  | yes                                                              |
    -| UseProviders.Secret              | Oauth Client Secret                                                                                                                                                                                                                               | yes                                                              |
    +| UseProviders.Key                 | Oauth Client key                                                                                                                                                                                                                                  | Yes                                                              |
    +| UseProviders.Secret              | Oauth Client Secret                                                                                                                                                                                                                               | Yes                                                              |
    Suggestion importance[1-10]: 7

    Why: The suggestion improves consistency in the documentation by standardizing the capitalization of the 'Required' field values, which enhances readability and maintains uniformity across the document. This is a minor but valuable enhancement to the documentation quality. The improved code accurately reflects the suggested change.

    7

    Copy link

    netlify bot commented Oct 30, 2024

    PS. Pls add /docs/nightly to the end of url

    Name Link
    🔨 Latest commit 438a829
    🔍 Latest deploy log https://app.netlify.com/sites/tyk-docs/deploys/67224afe86f16100089fc1f5
    😎 Deploy Preview https://deploy-preview-5681--tyk-docs.netlify.app
    📱 Preview on mobile
    Toggle QR Code...

    QR Code

    Use your smartphone camera to open QR code link.

    To edit notification comments on pull requests, go to your Netlify site configuration.

    Copy link

    netlify bot commented Oct 30, 2024

    PS. Pls add /docs/nightly to the end of url

    Name Link
    🔨 Latest commit 2305c9c
    🔍 Latest deploy log https://app.netlify.com/sites/tyk-docs/deploys/67372c902e24f600088e7724
    😎 Deploy Preview https://deploy-preview-5681--tyk-docs.netlify.app
    📱 Preview on mobile
    Toggle QR Code...

    QR Code

    Use your smartphone camera to open QR code link.

    To edit notification comments on pull requests, go to your Netlify site configuration.

    @sredxny sredxny marked this pull request as draft November 5, 2024 12:27
    @sredxny sredxny added the 1.6.0 TIB v1.6.0 label Nov 5, 2024
    @sredxny sredxny added 1.6.1 TIB and removed 1.6.0 TIB v1.6.0 labels Nov 8, 2024
    @sredxny sredxny changed the title Documented JWE config options [DX-1766] Documented JWE config options Nov 8, 2024
    @sredxny sredxny marked this pull request as ready for review November 8, 2024 14:32
    Copy link
    Contributor

    github-actions bot commented Nov 8, 2024

    Persistent review updated to latest commit d5d51c6

    @sredxny sredxny marked this pull request as draft November 8, 2024 15:37
    @sredxny sredxny marked this pull request as ready for review November 8, 2024 16:41
    Copy link
    Contributor

    github-actions bot commented Nov 8, 2024

    Persistent review updated to latest commit a140599

    Comment on lines +20 to +21
    - A certificate with a private key for Tyk (used to decrypt the ID token)
    - A public key file for the IdP (used to encrypt the ID token)
    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    These two steps does not provide much context on what exactly has to be done.

    Can you clarify the following:

    1. The algorithm to be used for asymmetric keys & it's size. RS-256, RS-512
    2. Will self-signed certificate work? Or an official certificate is required
    3. In which format the cerificate & other keys are expected. (.pem etc...)

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    @sharadregoti we do not limit to which certificates algos to use, you can use whatever you want 🤔 so not sure if we should mention this, this looks more like documentation for the certificate manager rather than TIB.
    #2 self signed cert will work.
    #3 The ones supported by the certificate manager

    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Got it, I'll rephrase the above.


    ##### 1. Prepare encryption keys
    1.1. Load the certificate with the private key into Tyk:
    - For embedded TIB in Dashboard: Use Tyk Dashboard's certificate manager
    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Add an image of dashboard showing the above.

    2.1. Create a new client in your IdP for Tyk Identity Broker

    ##### 3. Setup OIDC Profile
    3.1. Create a new TIB profile:
    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Add an image of dashboard for the same.

    ### JWE step-by-step guide

    #### Prerequisites
    - An Identity Provider (IdP) that supports JSON Web Encryption (JWE)
    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    We should write this document with a specific IDP, such as Keycloak, in mind. Currently, the information provided about the IDP is too generic. While users will adjust the steps according to their chosen IDP, we should at least develop this document with Keycloak or other relevant IDPs as references.

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Indeed, however currently there is not free software to test JWE, only keycloak but is broken. In order for us to test we had to built keycloack from source from a branch that is fixing their issues but not released yet. I consider that put screenshots or base the example on a third party app that is currently broken will not be 100% helpful as they will experience issues. Eg: I write the example using keycloack, then you in your local will follow those steps but it will not work because keycloak is broken... How do you think we should proceed with this one? knowing that at some point anyway keycloak will release the fix

    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    You can either use auth0—the free tier should suffice for the purpose of writing the article—or opt for keycloak. If you choose keycloak, we'll include a note detailing the steps we took to make it work and link to the relevant GitHub issues.

    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    If the steps and instructions can be applied for all Identity Provider (IdP) that supports JSON Web Encryption (JWE), we don't need to provide tool-specific guidance is it?

    I'm afraid we cannot and would not keep track of changes that happen on other tools. What is working today may be changed in another release if the instruction is too detailed.

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    @sharadregoti unfortunately the free tier of oauth0 doesnt allow us to enable JWE, its a regulated and paid feature (this was our nightmare to test the feature)

    1.2. Load the public key into your IdP for ID token encryption (process varies by IdP)

    ##### 2. Configure the Identity Provider
    2.1. Create a new client in your IdP for Tyk Identity Broker
    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Also add what things to be noted down after creating the client. Client ID, Client Secret.

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    wdym? which things are important to copy from the client? only client secret, cliend id and the wellknownopenidconfiguration url

    @sharadregoti sharadregoti mentioned this pull request Nov 14, 2024
    6 tasks
    Copy link
    Contributor

    @sharadregoti sharadregoti left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Thanks for creating .gif files. It's great. Only thing, I want to point out next time take full screen recording.

    ### JWE step-by-step guide

    #### Prerequisites
    - An Identity Provider (IdP) that supports JSON Web Encryption (JWE)
    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    You can either use auth0—the free tier should suffice for the purpose of writing the article—or opt for keycloak. If you choose keycloak, we'll include a note detailing the steps we took to make it work and link to the relevant GitHub issues.

    Comment on lines +20 to +21
    - A certificate with a private key for Tyk (used to decrypt the ID token)
    - A public key file for the IdP (used to encrypt the ID token)
    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Got it, I'll rephrase the above.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants