langfuse icon indicating copy to clipboard operation
langfuse copied to clipboard

Feature/Add KeyCloak authentication option

Open RTae opened this issue 1 year ago • 12 comments

What does this PR do?

Adding an SSO authentication option to support KeyCloak

Type of change

  • [x] Add Environment variable for KeyCloak authentication in web/src/env.mjs and .env.prod.example
  • [x] Add Keycloak provider schema type in ee/src/sso/types.ts
  • [x] Add a sign-in button for Keycloak when providing a Keycloak configuration in web/src/pages/auth/sign-in.tsx
  • [x] Add a function to push auth provider into list of auth provider when KeyCloak's configuration is set in web/src/server/auth.ts
  • [x] Add a logic to delete refresh_expires_in and not-before-policy before processing a linking account, since Keycloak returns payload data that is incompatible with current nextjs-auth schema.

Checklist

  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my PR needs changes to the documentation
  • I haven't added tests that prove my fix is effective or that my feature works
  • I haven't checked if new and existing unit tests pass locally with my changes

RTae avatar Aug 08 '24 10:08 RTae

Someone is attempting to deploy a commit to the langfuse Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Aug 08 '24 10:08 vercel[bot]

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Aug 08 '24 10:08 CLAassistant

related: #2846

marcklingen avatar Aug 08 '24 11:08 marcklingen

Hi, @marcklingen Do you have more comments on my PR? I'm not sure what's wrong with Snyk. It keeps giving me an error about a missing file. Could you point out the issue? I will fix it.

RTae avatar Aug 13 '24 05:08 RTae

Do you have a plan of when you will merge this PR? Thank u!

jmoratat avatar Aug 25 '24 22:08 jmoratat

Upvoting this issue, this would be very useful!

federico-pisanu avatar Aug 27 '24 07:08 federico-pisanu

What is missing for this to be accepted? Anyway I can help? i was going to try to create a PR for this, but I found this one and I don't know how to fix what's missing.

aleprj avatar Sep 11 '24 12:09 aleprj

What is missing for this to be accepted? Anyway I can help? i was going to try to create a PR for this, but I found this one and I don't know how to fix what's missing.

@aleprj Do you have any idea why CI is error ?

RTae avatar Sep 18 '24 09:09 RTae

@RTae @aleprj @marcklingen

Regarding the e2e test failing:

Run pnpm --filter=web exec playwright install --with-deps
  pnpm --filter=web exec playwright install --with-deps
  shell: /usr/bin/bash -e {0}
  env:
    PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
Installing dependencies...
Switching to root user to install dependencies...
Get:1 file:/etc/apt/apt-mirrors.txt Mirrorlist [14[2](https://github.com/langfuse/langfuse/actions/runs/10900105582/job/30246989076?pr=2866#step:10:2) B]
Hit:6 https://packages.microsoft.com/repos/azure-cli jammy InRelease
Get:7 https://packages.microsoft.com/ubuntu/22.04/prod jammy InRelease [[3](https://github.com/langfuse/langfuse/actions/runs/10900105582/job/30246989076?pr=2866#step:10:3)632 B]
Hit:2 http://azure.archive.ubuntu.com/ubuntu jammy InRelease
Get:3 http://azure.archive.ubuntu.com/ubuntu jammy-updates InRelease [128 kB]
Get:[4](https://github.com/langfuse/langfuse/actions/runs/10900105582/job/30246989076?pr=2866#step:10:4) http://azure.archive.ubuntu.com/ubuntu jammy-backports InRelease [127 kB]
Get:[5](https://github.com/langfuse/langfuse/actions/runs/10900105582/job/30246989076?pr=2866#step:10:5) http://azure.archive.ubuntu.com/ubuntu jammy-security InRelease [129 kB]
Ign:8 https://packages.microsoft.com/ubuntu/22.04/prod jammy/main arm[6](https://github.com/langfuse/langfuse/actions/runs/10900105582/job/30246989076?pr=2866#step:10:7)4 Packages
Get:9 http://azure.archive.ubuntu.com/ubuntu jammy-updates/main amd64 Packages [2058 kB]
Get:10 http://azure.archive.ubuntu.com/ubuntu jammy-updates/main Translation-en [355 kB]
Get:11 http://azure.archive.ubuntu.com/ubuntu jammy-updates/main amd64 c-n-f Metadata [1[7](https://github.com/langfuse/langfuse/actions/runs/10900105582/job/30246989076?pr=2866#step:10:8).8 kB]
Get:12 http://azure.archive.ubuntu.com/ubuntu jammy-updates/restricted amd64 Packages [2495 kB]
Get:13 http://azure.archive.ubuntu.com/ubuntu jammy-updates/restricted Translation-en [429 kB]
Get:14 http://azure.archive.ubuntu.com/ubuntu jammy-updates/universe amd64 Packages [1124 kB]
Get:15 http://azure.archive.ubuntu.com/ubuntu jammy-updates/universe Translation-en [261 kB]
Get:16 http://azure.archive.ubuntu.com/ubuntu jammy-updates/universe amd64 c-n-f Metadata [26.2 kB]
Get:17 http://azure.archive.ubuntu.com/ubuntu jammy-backports/universe amd64 Packages [2[8](https://github.com/langfuse/langfuse/actions/runs/10900105582/job/30246989076?pr=2866#step:10:9).8 kB]
Get:18 http://azure.archive.ubuntu.com/ubuntu jammy-backports/universe amd64 c-n-f Metadata [672 B]
Ign:27 https://packages.microsoft.com/ubuntu/22.04/prod jammy/main armhf Packages
Get:1[9](https://github.com/langfuse/langfuse/actions/runs/10900105582/job/30246989076?pr=2866#step:10:10) http://azure.archive.ubuntu.com/ubuntu jammy-security/main amd64 Packages [1806 kB]
Get:20 http://azure.archive.ubuntu.com/ubuntu jammy-security/main Translation-en [295 kB]
Get:21 http://azure.archive.ubuntu.com/ubuntu jammy-security/main amd64 c-n-f Metadata [13.3 kB]
Get:22 http://azure.archive.ubuntu.com/ubuntu jammy-security/restricted amd64 Packages [2377 kB]
Get:23 http://azure.archive.ubuntu.com/ubuntu jammy-security/restricted Translation-en [409 kB]
Get:24 http://azure.archive.ubuntu.com/ubuntu jammy-security/universe amd64 Packages [902 kB]
Get:25 http://azure.archive.ubuntu.com/ubuntu jammy-security/universe Translation-en [176 kB]
Get:26 http://azure.archive.ubuntu.com/ubuntu jammy-security/universe amd64 c-n-f Metadata [19.3 kB]
Ign:28 https://packages.microsoft.com/ubuntu/22.04/prod jammy/main amd64 Packages
Get:8 https://packages.microsoft.com/ubuntu/22.04/prod jammy/main arm64 Packages [43.0 kB]
Err:8 https://packages.microsoft.com/ubuntu/22.04/prod jammy/main arm64 Packages
  File has unexpected size (43334 != 43043). Mirror sync in progress? [IP: 13.[10](https://github.com/langfuse/langfuse/actions/runs/10900105582/job/30246989076?pr=2866#step:10:11)7.246.41 443]
  Hashes of expected file:
   - Filesize:43043 [weak]
   - SHA512:88b58c2d6a0cebf09639ba09d53536630cf29f7cce4e0f11d94bd5d4c550da8f5ddb2ecc68405b6b3306f3480a92bc25e3f7dcf4d63bcd529acb721d23b66a04
   - SHA256:6adc3ac2ed8c34f76a2199687e0dc23ca4f4e7f84bbc659b8d71d812424f8acb
  Release file created at: Thu, 12 Sep 2024 18:17:22 +0000
Get:27 https://packages.microsoft.com/ubuntu/22.04/prod jammy/main armhf Packages [15.3 kB]
Err:27 https://packages.microsoft.com/ubuntu/22.04/prod jammy/main armhf Packages
  
Get:28 https://packages.microsoft.com/ubuntu/22.04/prod jammy/main amd64 Packages [173 kB]
Err:28 https://packages.microsoft.com/ubuntu/22.04/prod jammy/main amd64 Packages
  
Fetched 13.2 MB in 2s (75[11](https://github.com/langfuse/langfuse/actions/runs/10900105582/job/30246989076?pr=2866#step:10:12) kB/s)
Reading package lists...
E: Failed to fetch https://packages.microsoft.com/ubuntu/22.04/prod/dists/jammy/main/binary-arm64/Packages.gz  File has unexpected size (43334 != 43043). Mirror sync in progress? [IP: 13.107.246.41 443]
   Hashes of expected file:
    - Filesize:43043 [weak]
    - SHA5[12](https://github.com/langfuse/langfuse/actions/runs/10900105582/job/30246989076?pr=2866#step:10:13):88b58c2d6a0cebf09639ba09d53536630cf29f7cce4e0f11d94bd5d4c550da8f5ddb2ecc68405b6b3306f3480a92bc25e3f7dcf4d63bcd529acb721d23b66a04
    - SHA256:6adc3ac2ed8c34f76a2199687e0dc23ca4f4e7f84bbc659b8d71d812424f8acb
   Release file created at: Thu, 12 Sep 2024 18:17:22 +0000
E: Failed to fetch https://packages.microsoft.com/ubuntu/22.04/prod/dists/jammy/main/binary-armhf/Packages.gz  
E: Failed to fetch https://packages.microsoft.com/ubuntu/22.04/prod/dists/jammy/main/binary-amd64/Packages.gz  
E: Some index files failed to download. They have been ignored, or old ones used instead.
Failed to install browsers
Error: Installation process exited with code: 100
undefined
/home/runner/work/langfuse/langfuse/web:
 ERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL  Command failed with exit code 1: playwright install --with-deps

This looks like an edge case that may be due to microsoft performing a sync and the files on the server don't match the expected hash. This can be resolved(ish) in several ways: simply run the CI again and/or add retries to the install steps, e.g.:

      - name: Install playwright
        run: |
          for i in {1..3}; do
            pnpm --filter=web exec playwright install --with-deps && break || sleep 10
          done

Alternative and additionally, it may be worth while to switch the ubuntu runner to a more stable version:

In this file change all:

runs-on: ubuntu-latest to runs-on: ubuntu-22.04

for better /more stable CI-experience.


As for the Snyk CI error:

1s
Run github/codeql-action/upload-sarif@v3
Error: Path does not exist: snyk.sarif

I have no clue.

I would really like this feature to be deployed. Anything i can do to help?

Luca05 avatar Sep 22 '24 11:09 Luca05

What is missing for this to be accepted? Anyway I can help? i was going to try to create a PR for this, but I found this one and I don't know how to fix what's missing.

@aleprj Do you have any idea why CI is error ?

e2e-test is fixed (by running it again by @RTae after merging main into feature branch)


@marcklingen:

Regarding Snyk, it has an authentication problem: image

and it cannot upload the results to github.

Error: Path does not exist: snyk.sarif

Should this CI-step even be run on a feature branch or only main? see PR

Luca05 avatar Sep 22 '24 12:09 Luca05

Is there any reason not to pr?

wolfdate25 avatar Sep 30 '24 00:09 wolfdate25

@marcklingen Any update?? Thanks!

ghost avatar Oct 07 '24 11:10 ghost

@RTae any plans to merge this pr ? as we need this functionality

sanknoorsachin avatar Oct 28 '24 12:10 sanknoorsachin

@sanknoorsachin , I am not a maintainer for this repo. Also, I am still looking for a maintainer to merge this PR.

cc. @marcklingen

RTae avatar Oct 28 '24 12:10 RTae

We are also interested in this feature as we run into the same issue as https://github.com/langfuse/langfuse/issues/3716 and we are doing the keycloak workaround because the proxy is not supported for entra id (https://github.com/langfuse/langfuse/issues/3911).

@marcklingen any update on this when to merge / approve or what is missing for this case?

schnaker85 avatar Nov 07 '24 12:11 schnaker85

@marcklingen is there a reason why this PR is no longer looked at? I would have another suggestion for a possible other solution using the custom Auth. The idea is as following to use keycloak auth.

  • use AUTH_CUSTOM_XXX mode with the provided variables
  • add a new env variable AUTH_CUSTOM_REMOVE_DATA_FIELDS (or something you define as self-describing).
  • specify in that field: AUTH_CUSTOM_REMOVE_DATA_FIELDS=refresh_expires_in,not-before-policy
  • add in the auth.ts the following code to use the fields

const extendedPrismaAdapter: Adapter = {
  ...prismaAdapter,
 
async linkAccount(data) {
    if (!prismaAdapter.linkAccount)
      throw new Error("linkAccount not implemented");

    
    if (env.AUTH_CUSTOM_REMOVE_DATA_FIELDS) {
      const removeDataFields = env.AUTH_CUSTOM_REMOVE_DATA_FIELDS.split(',');
      for (field in removeDataFields) {
           delete data[field];
    }

    await prismaAdapter.linkAccount(data);
  },
};

something like that, this would be generally helpful and would be an easy fix.

What do you think about something like that and would you be able to bring this in the next release?

schnaker85 avatar Nov 13 '24 12:11 schnaker85

Just released this as part of v3.0.0-rc.3

Thanks again, everyone, for your contributions to this PR. I apologize for my delay in responding. As mentioned earlier, I accidentally muted this email thread, which caused me to miss the updates on this PR. Sorry about this!

marcklingen avatar Dec 03 '24 17:12 marcklingen

Just brought this to v2 branch as well to allow for Keycloak usage without having to upgrade to v3 immediately, the published container image will be available in an hour

https://github.com/langfuse/langfuse/releases/tag/v2.93.3

marcklingen avatar Dec 03 '24 22:12 marcklingen

Thanks again to @RTae and everyone who contributed to this

marcklingen avatar Dec 03 '24 22:12 marcklingen

Thanks @RTae and @marcklingen , works very well with keycloak now, just went to PRD with the newly available working config 🚀

schnaker85 avatar Dec 05 '24 09:12 schnaker85