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

Support for Cesium ion servers running in single-user authentication mode. #841

Merged
merged 14 commits into from
Apr 26, 2024

Conversation

azrogers
Copy link
Contributor

@azrogers azrogers commented Apr 2, 2024

Out of the box, Cesium ion Self-Hosted is set-up in the single-user authentication mode, which means that it has no accounts or tokens - any request that can reach the server is considered valid. This is a problem for cesium-native and its integrations, which expect Cesium ion to always require a token.

This change is a necessary part of adding this support. It changes cesium-native to allow requesting the active authentication mode from the /appData endpoint, and allows Unity and Unreal integrations to access the current authentication mode. It also provides dummy responses for the /me and /tokens endpoints when running in single-user mode, as these endpoints don't exist in the single-user configuration.

*
* @return A future that resolves to the application information.
*/
static CesiumAsync::Future<Response<ApplicationData>> appData(
Copy link
Contributor

Choose a reason for hiding this comment

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

Kinda think the verb form "getAppData" would be more descriptive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would make sense, but I wanted to keep the naming consistent with the rest of the methods. In this class, all the get* methods return private members of the class, whereas all the methods calling endpoints are named with that endpoint name (me, assets, defaults, etc).

Copy link
Contributor

@j9liu j9liu 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 to me @azrogers! Not super familiar with the ion code myself, but with respect to everything else in there, it makes sense to me 😄

Just a few nitpick comments. Could you also merge with main and update CHANGES.md for this PR?

CesiumIonClient/include/CesiumIonClient/ApplicationData.h Outdated Show resolved Hide resolved
CesiumIonClient/include/CesiumIonClient/ApplicationData.h Outdated Show resolved Hide resolved
CesiumIonClient/include/CesiumIonClient/ApplicationData.h Outdated Show resolved Hide resolved
CesiumIonClient/src/Connection.cpp Show resolved Hide resolved
@j9liu
Copy link
Contributor

j9liu commented Apr 26, 2024

Thanks @azrogers !

@j9liu j9liu merged commit fdf90e1 into main Apr 26, 2024
1 of 14 checks passed
@j9liu j9liu deleted the ion-single-user branch April 26, 2024 20:58
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