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

Hardcoded oauth config #9484

Closed
jnweiger opened this issue Mar 5, 2022 · 6 comments
Closed

Hardcoded oauth config #9484

jnweiger opened this issue Mar 5, 2022 · 6 comments
Assignees
Labels

Comments

@jnweiger
Copy link
Contributor

jnweiger commented Mar 5, 2022

Seen in client 2.10.0 with the openidconnect app 2.1.1 rc1 running on server 10.9.1 -- connected to Azure

This is a followup on https://github.com/owncloud/enterprise/issues/4386

String Theme::openIdConnectPrompt() const
{
    return QStringLiteral("select_account consent");
}

as e.g. seen in https://github.com/owncloud/client/blob/master/src/libsync/theme.cpp#L646
is invalid with (at least my playground in) Azure: "AADSTS90023: Unsupported 'prompt' value."

Azure works, when it is only consent, as shown in my original https://github.com/owncloud/enterprise/issues/4386#issuecomment-799865744 research.

I am aware that differnt brandings can overwrite this in their theme. But I believe the value seen here, and used by the unbranded ownCloud client, and testpilotcloud and btr branding should a valid default.

For best flexibility, I'd suggest to allow changing this string without recompiling, e.g. via env (or maybe *.cfg )

QString openIdConnectPrompt() const
{
    return qEnvironmentVariable("OWNCLOUD_OPENIDCONNECT_PROMPT", QStringLiteral("consent"));
}

@jnweiger jnweiger changed the title [QA] openIDconnectPrompt default does not work with Azure [QA] openIdConnectPrompt default does not work with Azure Mar 5, 2022
@michaelstingl
Copy link
Contributor

@IljaN @ChrisEdS @wkloucek what would be your recommendation for the unbranded client? I don't want to break other IdP's.

/cc @TheOneRing

@TheOneRing
Copy link
Member

Related: #9339

@TheOneRing TheOneRing changed the title [QA] openIdConnectPrompt default does not work with Azure Hardcoded oauth config Mar 7, 2022
@TheOneRing
Copy link
Member

Different idps require different parameter.
As far as we know there is no default we can use for all idps.

I'm against adding any more environment variables, #9339 suggest adding it to the config.

The upcoming new theming support could also make it easier to provide the idp settings in a branding.

Configurable oauth settings require research regarding security.

@jnweiger
Copy link
Contributor Author

jnweiger commented Mar 7, 2022

We already broke azure by adding select_account -- which IDP would break, when we remove that again?

Config option in the config is okay with me too. I only used env here for simplicity.

@github-actions
Copy link

github-actions bot commented Apr 7, 2022

This issue was marked stale because it has been open for 30 days with no activity. Remove the stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Apr 7, 2022
@github-actions
Copy link

The issue was marked as stale for 7 days and closed automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants