Skip to content
This repository was archived by the owner on Apr 19, 2022. It is now read-only.

Cannot fetch a user specific theme for various reasons#37

Draft
butonic wants to merge 1 commit intoowncloud:masterfrom
butonic:load-theme-from-accounts-service
Draft

Cannot fetch a user specific theme for various reasons#37
butonic wants to merge 1 commit intoowncloud:masterfrom
butonic:load-theme-from-accounts-service

Conversation

@butonic
Copy link
Copy Markdown
Contributor

@butonic butonic commented Jan 31, 2020

I wanted to use the account service to dynamically adjist the theme, based on who is logged in.

This PR serves to document several problems we should discuss on how to address them:

  1. the config.json contains the theme ... which can only serve as a default theme, because the user is not yet logged in. AFAICT we can keep this theme as the default theme, but phoenix will have to fetch user specific settings in a request that happens after the user logged in
  2. the config.json endpoint is not using the authentication middleware (which makes sense, because it is fetched before logging in) so we cannot access the oidc claims, even if the user is already logged in.

The way I set up the micro client to look up the accounts service and make a call to fetch the account Record serves as an example. It should be refactored into a separate func of course. In any case I'll put this here for others to learn.

@update-docs
Copy link
Copy Markdown

update-docs bot commented Jan 31, 2020

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@ownclouders
Copy link
Copy Markdown

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- pkg/service/v0/service.go  2
         

See the complete overview on Codacy

@tboerger
Copy link
Copy Markdown
Contributor

tboerger commented Feb 1, 2020

IMHO it doesn't make sense to change the config json based on being logged in.

@DeepDiver1975
Copy link
Copy Markdown
Member

Where is this requirement coming from? We had long req analysis phases on what theming should do. User based theming was never mentioned.

@butonic
Copy link
Copy Markdown
Contributor Author

butonic commented Feb 4, 2020

I agree that config.json should not contain user specific configuration. While user individual themes are out of scope for now we want to use the accounts service to determine which apps to load for a user. Not all users will be granted access to all applications, eg. to allow premium accounts or A/B testing of different versions of external apps.

I used themes to provide a more simple example. AFAICT the correct enhancement is to read the list of enabled apps from the /ocs/v1.php/cloud/user endpoint. Or some graph api equivalent in the future.

Does that make more sense?

@tboerger
Copy link
Copy Markdown
Contributor

tboerger commented Feb 4, 2020

It makes sense to retrieve the list of allowed apps from some api endpoint, but not from the config json of Phoenix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants