eikek / sharry

Sharry is a self-hosted file sharing web application.
https://eikek.github.io/sharry
GNU General Public License v3.0
890 stars 56 forks source link

Azure AD scope is duplicated. #1270

Closed albertbern closed 10 months ago

albertbern commented 11 months ago

Azure gives error: The request is not properly formatted. The parameter 'scope' is duplicated.

Argument scope= is duplicated in URL:

" https://login.microsoftonline.com/TENANT/oauth2/v2.0/authorize?scope=openid&client_id=CLIENTID&scope=openid&redirect_uri=https%3A//DOM/api/v2/open/auth/oauth/aad/resume&response_type=code&state=GOOMOO"

If I comment scope=''" from AAD configuration sharry-restserver won`t start:

Dec 12 11:53:42 servername sharry-restserver[425401]: 2023.12.12 11:53:42:105 io-compute-4 INFO sharry.restserver.Main.run:35 Dec 12 11:53:42 servername sharry-restserver[425401]: Using config file from system properties: /usr/share/sharry-restserver/bin/../conf/sharry.conf Dec 12 11:53:43 servername sharry-restserver[425401]: pureconfig.error.ConfigReaderException: Cannot convert configuration to a sharry.restserver.config.Config. Failures are: Dec 12 11:53:43 servername sharry-restserver[425401]: at 'sharry.restserver.backend.auth.oauth.2': Dec 12 11:53:43 servername sharry-restserver[425401]: - (/usr/share/sharry-restserver/bin/../conf/sharry.conf: 263) Key not found: 'scope'. Dec 12 11:53:43 servername sharry-restserver[425401]: at pureconfig.ConfigSource.loadOrThrow(ConfigSource.scala:81) Dec 12 11:53:43 servername sharry-restserver[425401]: at pureconfig.ConfigSource.loadOrThrow$(ConfigSource.scala:78) Dec 12 11:53:43 servername sharry-restserver[425401]: at pureconfig.ConfigSource$$anon$2.loadOrThrow(ConfigSource.scala:332) Dec 12 11:53:43 servername sharry-restserver[425401]: at sharry.restserver.config.ConfigFile$.loadConfig(ConfigFile.scala:20) Dec 12 11:53:43 servername sharry-restserver[425401]: at sharry.restserver.Main$.$anonfun$run$5(Main.scala:41) Dec 12 11:53:43 servername sharry-restserver[425401]: at apply @ sharry.restserver.Main$.run(Main.scala:20) Dec 12 11:53:43 servername sharry-restserver[425401]: at flatMap @ sharry.restserver.Main$.run(Main.scala:19) Dec 12 11:53:43 servername sharry-restserver[425401]: at make @ sharry.common.ThreadFactories$.executorResource(ThreadFactories.scala:42) Dec 12 11:53:43 servername sharry-restserver[425401]: at make @ sharry.common.ThreadFactories$.executorResource(ThreadFactories.scala:42) Dec 12 11:53:43 servername sharry-restserver[425401]: at main$ @ sharry.restserver.Main$.main(Main.scala:11) Dec 12 11:53:43 servername systemd[1]: sharry-restserver.service: Main process exited, code=exited, status=1/FAILURE Dec 12 11:53:43 servername systemd[1]: sharry-restserver.service: Failed with result 'exit-code'.

albertbern commented 11 months ago

Old version 1.11.0 has no such bug.

eikek commented 11 months ago

Thanks for reporting!

albertbern commented 10 months ago

thank you very much!

lauer commented 10 months ago

Thank you, for the great app. Can you provide a hotfix for this bug, or when do you plan fix and release this (1.13.0)?

eikek commented 10 months ago

I was planning to look at it in the next days. (no guarantees though :))

But currently I cannot reproduce it on the current master branch. Can someone with that issue share your config? And/or try with the current snapshot release? 🙏🏼

lauer commented 10 months ago

I can confirm, that it works in 1.11.0, but not in 1.12.1. I would love to try nightly, but I get a database error when I boot it. Guess its not happy with H2.

Caused by: org.h2.jdbc.JdbcSQLNonTransientConnectionException: Unsupported database file version or invalid file header in file "/dev/db/sharry-database.db.mv.db" [90048-224]
Caused by: org.h2.mvstore.MVStoreException: The write format 2 is smaller than the supported format 3 [2.2.224/5]

There were no problems in switching between 1.11.0 and 1.12.1.

According to the documentation, there is only an easy way to migrate files between databases, not users. My files are stored in a mounted filestorage. I could try reinstall with postgresql, since my installation is pretty new. But others might face other problems, so I will wait if I need to test something for you first in a nightly fix build?

eikek commented 10 months ago

I think this is due to an upgrade of H2. You need to either start with a new db or migrate. Or you could use the docker images that are setup for postgres

eikek commented 10 months ago

Since I don't have any Azure setup I can't test this issue "for real". Right now I can't reproduce it via other openId providers, though. It would also help to see your sharry configuration regarding this provider. In 1.12 the scope parameter was added to the config. Before there was no such thing in the sharry config. It could be that if you used sharry before, you might have added this param yourself in the configs authorize url? I know not likely, but it is an option :)

lauer commented 10 months ago

I run my setup in a kubernetes setup, but I am able to create a postgres installation on there. It was more, if you want to support H2 database.

So my config looks like this. But I might switch to a keycloak installation anyway. If I remove the scope part, I get the same error as OP

oauth = [
                        {
                          enabled = true
                          id = "aad"
                          name = "Azure AD"
                          icon = "fab fa-microsoft"
                          scope = "openid"
                          authorize-url = "https://login.microsoftonline.com/<snip>/oauth2/v2.0/authorize?scope=openid"
                          token-url = "https://login.microsoftonline.com/<snip>/oauth2/v2.0/token"
                          user-url = "https://graph.microsoft.com/oidc/userinfo"
                          user-id-key = "email"
                          user-email-key = "email"
                          client-id = "<snip>"
                          client-secret = "<snip>"
                        }

              ]
lauer commented 10 months ago

btw, you can create a free Microsoft Entra tenant for testing - https://learn.microsoft.com/en-us/entra/verified-id/how-to-create-a-free-developer-account

eikek commented 10 months ago

authorize-url = "https://login.microsoftonline.com//oauth2/v2.0/authorize?scope=openid"

Ah see - you already added the scope via the authorize url. I suppose this comes from a version prior to 1.12 where there was no scope possible to define in the config. You need to remove this custom parameter now.

lauer commented 10 months ago

Ohh, How stupid I am. Just followed the documentation too much. Of cause it works now :)

You should maybe remove it from the example here: https://eikek.github.io/sharry/doc/configure

eikek commented 10 months ago

Oh right, sorry for that confusion - didn't realize it's so wrong in the docs! Will remove it asap 😄