Allow choosing the OIDC remote user claim and scopes to request from the identity provider

[otaconix]

Changes proposed in this pull request:

• Allow setting the OIDC_SCOPES environment variable to control what scopes to request from the OIDC identity provider. This defaults to openid.
• Allow setting the OIDC_REMOTE_USER_CLAIM environment to control what claim to use as the username. This defaults to preferred_username.
• Document these environment variables.

How to test the feature manually:

1. I personally needed this to integrate with Authentik, so configure that, create an OIDC provider and an application.
2. Configure a FreshRSS Debian docker container (I used Docker/Dockerfile) to use Authentik as the OIDC identity provider. Use the following environment variables besides the ones needed to get OIDC in FreshRSS:
   • OIDC_SCOPES=openid profile (profile is required to get the preferred_username claim)
3. Open the FreshRSS instance. You should be redirected to Authentik to login, then go back to FreshRSS and be asked to setup the new instance. If it doesn't work, you'll be greeted with some error page. If it works, you can setup the instance.

Pull request checklist:

• clear commit messages
• code manually tested
• unit tests written (optional if too hard)
• documentation updated

[Alkarex]

Follow-up of #5351
Ping @aaronschif

[otaconix]

Alright, I cleaned up my commits a little, and finally got something to work for using default values for unset environment variables. The end-result may not be very pretty, if anyone more knowledgeable than me has suggestions to make this better, I'm all ears!

[Alkarex]

Can that be put inside the <IfDefine OIDC_ENABLED> block?

[otaconix]

It certainly can. I put those there because I considered using the same trick to get rid of the -D OIDC_ENABLED in the Dockerfiles, but decided not to so it wouldn't further "pollute" the PR. I can still do either (move these Define into the OIDC_ENABLED conditional, or use the trick to get rid of the conditional in the Dockerfiles). I'll leave that to you.

[Alkarex]

I have not fully understood your response :-)

If possible, I would suggest the following:

<IfDefine OIDC_ENABLED>
	<IfModule !auth_openidc_module>
		Error "The auth_openidc_module is not available. Install it or unset environment variable OIDC_ENABLED."
	</IfModule>

	# Workaround to be able to check whether an OS environment variable is set
	# https://serverfault.com/questions/1022233/using-ifdefine-with-environment-variables/1022234#1022234
	Define VStart "${"
	Define VEnd "}"

	OIDCProviderMetadataURL ${OIDC_PROVIDER_METADATA_URL}
	OIDCClientID ${OIDC_CLIENT_ID}
	OIDCClientSecret ${OIDC_CLIENT_SECRET}

	OIDCRedirectURI /i/oidc/
	OIDCCryptoPassphrase ${OIDC_CLIENT_CRYPTO_KEY}

	Define "Test_${OIDC_REMOTE_USER_CLAIM}"
	<IfDefine Test_${VStart}OIDC_REMOTE_USER_CLAIM${VEnd}>
		OIDCRemoteUserClaim preferred_username
	</IfDefine>
	<IfDefine !Test_${VStart}OIDC_REMOTE_USER_CLAIM${VEnd}>
		OIDCRemoteUserClaim "${OIDC_REMOTE_USER_CLAIM}"
	</IfDefine>
	Define "Test_${OIDC_SCOPES}"
	<IfDefine Test_${VStart}OIDC_SCOPES${VEnd}>
		OIDCScope openid
	</IfDefine>
	<IfDefine !Test_${VStart}OIDC_SCOPES${VEnd}>
		OIDCScope "${OIDC_SCOPES}"
	</IfDefine>

	OIDCRefreshAccessTokenBeforeExpiry 30
</IfDefine>

[otaconix]

I have not fully understood your response :-)

Apologies, I don't know how best to express this without actually showing what I mean in a diff. But I was referring to this:

FreshRSS/Docker/Dockerfile

Line 68 in 666e951

exec apache2 -D FOREGROUND $([ -n "$OIDC_ENABLED" ] && [ "$OIDC_ENABLED" -ne 0 ] && echo '-D OIDC_ENABLED')

The OIDC_ENABLED variable is defined on the commandline inside each of the Dockerfiles, and it doesn't need to be if we use the same pattern for checking whether an environment variable is set or not.

I applied your suggestion anyway. Let me know if there's anything else 👍

[Alkarex]

This OIDC_ENABLED variable is special in the sense that, as far as I can see, it is the only one that changes the Apache command line (we want to add a -D OIDC_ENABLED), so I do not believe it is the same situation than the other environment variables

[otaconix]

But why do you want to add a -D OIDC_ENABLED? Instead (and I'm not saying this is winning any beauty contests, but still), it could've been done like so (without the -D OIDC_ENABLED):

Define VStart "${"
Define VEnd "}"
Define "Test_${OIDC_ENABLED}"
<IfDefine !Test_${VStart}OIDC_ENABLED${VEnd}>
   # [...]
</IfDefine>

IMO, Apache's config doesn't lend itself well to this kind of runtime logic. Maybe some kind of templating solution would be better, but then you'd have one more moving piece in your docker image. I'm not sure what the best solution would be, honestly. What you have now works fine, so just keep that if you want 🙂

[Alkarex]

Looks all good to me now 👍🏻

[Alkarex]

@otaconix Please add a line for you in https://github.com/FreshRSS/FreshRSS/blob/edge/CREDITS.md