Skip to content

Conversation

@trmartin4
Copy link
Member

@trmartin4 trmartin4 commented Nov 26, 2025

📔 Objective

With the removal of the free Developer preview in Okta, we have moved away from that being an option for local SSO development.

This PR consolidates the SSO docs into a single page since there are no more options other than the local SAML configuration.

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation
    team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed
    issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Nov 26, 2025

Deploying contributing-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 890a219
Status: ✅  Deploy successful!
Preview URL: https://bd5aa3f7.contributing-docs.pages.dev
Branch Preview URL: https://remove-okta-sso-docs.contributing-docs.pages.dev

View logs

@github-actions
Copy link
Contributor

github-actions bot commented Nov 26, 2025

Logo
Checkmarx One – Scan Summary & Details296f42d2-2570-4d18-9252-e469783d5e50

Great job! No new security vulnerabilities introduced in this pull request

@trmartin4 trmartin4 marked this pull request as ready for review November 26, 2025 23:10
@trmartin4 trmartin4 requested review from a team as code owners November 26, 2025 23:10
@trmartin4 trmartin4 marked this pull request as draft November 26, 2025 23:11
@claude
Copy link

claude bot commented Nov 26, 2025

Claude finished @trmartin4's task —— View job


Review Complete

  • Evaluate PR title and description
  • Read changed files
  • Check documentation style compliance
  • Review content accuracy and clarity
  • Post comprehensive review

Summary

This PR successfully consolidates SSO documentation into a single page following Okta's removal of their free Developer preview. The documentation is well-structured and clear.

Findings

Finding 1: PR description and title are clear and well-written, with appropriate context about why this change was made.

Finding 2: The consolidation effectively merges local.md content into index.md, eliminating the now-unnecessary choice between Okta and local IdP.

Finding 3: Documentation follows the repository style guide with numbered instructions, code blocks, and proper formatting.

Finding 4: Custom word removal (oktapreview) is appropriate since Okta-specific documentation has been removed.

Finding 5: The "Desktop client" section was properly restructured from a standalone note into a full troubleshooting entry with detailed workaround steps.

Additional Notes

  • Two inline comments posted with minor suggestions for improved clarity
  • All frontmatter and conditional content tags remain appropriate (Bitwarden-only access)
  • File deletions are justified and complete (no orphaned references detected)
  • The consolidation improves maintainability by reducing documentation surface area

Good work on maintaining consistent formatting and comprehensive troubleshooting coverage.


@trmartin4 trmartin4 removed the request for review from a team November 26, 2025 23:12
@trmartin4 trmartin4 marked this pull request as ready for review November 26, 2025 23:12
[http://localhost:8090/simplesaml](http://localhost:8090/simplesaml) and clicking Authentication
→ Test Configured Authentication Sources → `example-userpass`. You should be able to log in with
the users you’ve configured.

Copy link

Choose a reason for hiding this comment

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

⚠️ Minor inconsistency in navigation instructions

The navigation path uses "→" (arrow) here, but step 88 above uses "clicking Authentication → Test Configured..." which is consistent. However, the deleted local.md file used commas in the path: "Authentication → test configured authentication sources → example-userpass" (lowercase "test").

The current version capitalizes "Test" which is more consistent with UI styling, but should we standardize whether we use arrows or commas for multi-level navigation paths?

- X509 Public Certificate
- Get this by opening a new tab and navigating to the Entity ID URL above. It will open (or
download) an XML file. Copy and paste the value _between_ the `<ds:X509Certificate>` tags
(it should look like a base64-encoded string).
Copy link

Choose a reason for hiding this comment

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

🎨 Consider clarifying "between" for better precision

The instruction says to copy the value "between" the tags, which is correct. However, the previous version in local.md used similar wording. For absolute clarity with technical users, consider: "Copy and paste the certificate value contained within the <ds:X509Certificate> tags (not including the opening and closing tags themselves)."

This removes any ambiguity about whether "between" includes the tags or not.

@trmartin4 trmartin4 merged commit fc8ca68 into main Dec 1, 2025
29 of 36 checks passed
@trmartin4 trmartin4 deleted the remove-okta-sso-docs branch December 1, 2025 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants