Skip to content

Conversation

@florindragos
Copy link
Contributor

use base64 encoded IDs to support any whitespaces

@florindragos florindragos force-pushed the docs branch 2 times, most recently from a5e0e08 to 306f505 Compare April 28, 2025 12:53
@florindragos florindragos requested review from Copilot and ronenh April 29, 2025 14:44
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces documentation updates for Okta and Entra ID integration and modifies the SCIM-related tests and conversion logic to support base64-encoded IDs for handling whitespaces. Key changes include:

  • Updating test cases in pkg/test/scim_test.go to use dynamic IDs returned from create endpoints.
  • Renaming functions and modifying transformation calls across user and group handlers and converter functions to incorporate base64 encoding.
  • Enhancing documentation in the docs folder and updating configuration and template loading in the application.

Reviewed Changes

Copilot reviewed 18 out of 23 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/test/scim_test.go Updated test cases to retrieve IDs dynamically and validate responses using base64 IDs.
pkg/test/assets/assets.go Added group asset and renamed Patch function to PatchOp to support new testing logic.
pkg/config/config.go Introduced TemplateFile to allow custom template configuration.
pkg/app/run.go Added logic to load a template file if specified.
docs/okta.md & docs/entra-id.md Added and updated documentation for Okta and Entra ID SCIM integration.
common/handlers/* (users/groups) Updated transformation calls by passing dynamic IDs rather than hardcoded values.
common/convert/* Updated conversion functions and tests to use base64 encoded IDs.
common/assets.go Modified the default template loader to support the new transform configuration.
README.md Updated provisioning documentation and clarified relation mappings.
Files not reviewed (5)
  • common/assets/template.tmpl: Language not supported
  • common/assets/users-groups.tmpl: Language not supported
  • common/assets/users.tmpl: Language not supported
  • pkg/test/assets/data/morty.json: Language not supported
  • pkg/test/assets/data/rick.json: Language not supported

@ronenh
Copy link
Contributor

ronenh commented Apr 29, 2025

I think this branch needs a rebase

edit: it doesn't. my mistake

Copy link
Contributor

@ronenh ronenh left a comment

Choose a reason for hiding this comment

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

Looks good. Just fix the typos found by Copilot.

Base automatically changed from common to main April 29, 2025 15:08
@florindragos
Copy link
Contributor Author

I think this branch needs a rebase

edit: it doesn't. my mistake

I did a rebase after squashing the other PR

@florindragos florindragos merged commit 95ad4f4 into main Apr 30, 2025
5 checks passed
@florindragos florindragos deleted the docs branch April 30, 2025 15:38
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