Skip to content

Support iTwin share keys #12530

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

Merged
merged 8 commits into from
Mar 30, 2025
Merged

Support iTwin share keys #12530

merged 8 commits into from
Mar 30, 2025

Conversation

jjspace
Copy link
Contributor

@jjspace jjspace commented Mar 19, 2025

Description

  • Add ITwinPlatform.defaultShareKey to support using the iTwin Share API for public access
    • This value will override the ITwinPlatform.defaultAccessToken if it's set
    • Currently this value must be sent using Basic auth instead of Bearer so I added a helper function for generating the Auth headers
  • Both iTwin sandcastles have been updated to use share keys and no longer need the ion token route

Issue number and link

No issue

Testing plan

Author checklist

  • I have submitted a Contributor License Agreement
  • I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change
  • I have added or updated unit tests to ensure consistent code coverage
  • I have updated the inline documentation, and included code examples where relevant
  • I have performed a self-review of my code

@jjspace jjspace requested a review from ggetz March 19, 2025 16:04
Copy link

Thank you for the pull request, @jjspace!

✅ We can confirm we have a CLA on file for you.

Copy link
Contributor

@ggetz ggetz left a comment

Choose a reason for hiding this comment

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

Thanks @jjspace! Just those two notes.

@jjspace
Copy link
Contributor Author

jjspace commented Mar 25, 2025

Thanks @ggetz, updated


Cesium.ITwinPlatform.defaultAccessToken = token;
// If you do use oauth for user login set:
// Cesium.ITwinPlatform.defaultAccessToken = 'your token'
Copy link
Member

Choose a reason for hiding this comment

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

@jjspace Do we need to have this here? Perhaps an update can point to the tutorial and developer documentation when ready. Same comment for the other sandcastle too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shehzan10 We can certainly add more docs or tutorials later but I don't think there's an issue with providing this information in multiple locations. These sandcastles may end up being one of the main ways to quick reference how to interact with iTwin data in CesiumJS. I think it's worth keeping these 2 lines just for convenience/reference

Copy link
Member

Choose a reason for hiding this comment

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

@jjspace With the goal being to introduce this to the Cesium community via CesiumJS first, IMO its important to keep it simple. By having the comment If you do use oauth for user login set, the statement is vauge-ish because OAuth login has a very specific meaning. So either:

  1. We remove this comment and instead point to the tutorials/documentation when ready.
  2. Make the comment more accurate, yet simple to understand and allows the developer on which approach to use.

Open to other suggestions as well.

Choose a reason for hiding this comment

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

I agree with @jjspace here, I think its nice to show users that they're able to use either form of authentication to use the sandcastle demo -- sharing or oauth. If you'd like to add more detail to suggesting oauth, you could point them do our oauth docs: https://developer.bentley.com/apis/overview/authorization/

Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to then have a comment like:

// For alternative forms of authentication you can use, visit https://developer.bentley.com/apis/overview/authorization/.

And combine this with line 35?

@ggetz
Copy link
Contributor

ggetz commented Mar 30, 2025

Thanks @jjspace, as well as @shehzan10 and @davidhjones for your feedback!

@ggetz ggetz added this pull request to the merge queue Mar 30, 2025
Merged via the queue into main with commit a396f7e Mar 30, 2025
9 checks passed
@ggetz ggetz deleted the itwin-share-key branch March 30, 2025 15:42
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.

4 participants