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.

Sign up for GitHub

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

Jump to bottom

add GET /api/v1/admin/backend_settings/{full/public} endpoint for api-clients #830

Merged
merged 3 commits into from Jan 19, 2023
Merged

add GET /api/v1/admin/backend_settings/{full/public} endpoint for api-clients #830

merged 3 commits into from Jan 19, 2023

Conversation

andreaskoepf
Copy link
Collaborator

@andreaskoepf andreaskoepf commented Jan 18, 2023 •
edited

/api/v1/admin/backend_settings/public -> all clients
/api/v1/admin/backend_settings/full -> only accessible for trusted api clients

solves #829
related to #822

@andreaskoepf andreaskoepf requested a review from yk as a code owner January 18, 2023 21:58
@andreaskoepf andreaskoepf changed the title add GET /api/v1/admin/backend_settings endpoint for trusted api-clients add GET /api/v1/admin/backend_settings/{full/public} endpoint for trusted api-clients Jan 18, 2023
@andreaskoepf andreaskoepf changed the title add GET /api/v1/admin/backend_settings/{full/public} endpoint for trusted api-clients add GET /api/v1/admin/backend_settings/{full/public} endpoint for api-clients Jan 18, 2023
@andreaskoepf andreaskoepf mentioned this pull request Jan 18, 2023
Closed
@andreaskoepf andreaskoepf linked an issue Jan 18, 2023 that may be closed by this pull request
Add configuration endpoint for trusted-api_clients #829
Closed
yk
yk approved these changes Jan 19, 2023
View reviewed changes
Copy link
Collaborator

@yk yk left a comment

Choose a reason for hiding this comment

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

lgtm, but probably a better way would be to have settings inherit from publicsettings and amend the private properties, then just specify publicsettings as response model, no conversion needed

@andreaskoepf
Copy link
Collaborator Author

lgtm, but probably a better way would be to have settings inherit from publicsettings and amend the private properties, then just specify publicsettings as response model, no conversion needed

I thought about that too .. but it gives less control over what should be handed out, in general I think exposing in internal settings object like it is done for the trusted-endoint should be an exception .. for the same reason that we normally don't directly return db-models but convert them into response-models which are sent back via json/http.

@andreaskoepf andreaskoepf enabled auto-merge (squash) January 19, 2023 11:46
@andreaskoepf andreaskoepf merged commit ef8a00e into main Jan 19, 2023
@andreaskoepf andreaskoepf deleted the 829_add_get_configuration_endpoint branch January 19, 2023 11:48
andreaskoepf added a commit that referenced this pull request Jan 19, 2023
@andreaskoepf
add GET /api/v1/admin/backend_settings/{full/public} endpoint for api…
be9ccb6 
…-clients (#830)

* add GET /api/v1/admin/backend_settings endpoint for trusted api-clients

* add backend_settings/public for untrusted api_clients
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yk yk yk approved these changes

Assignees
No one assigned
Labels
backend
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add configuration endpoint for trusted-api_clients
3 participants
@andreaskoepf @yk @fozziethebeat