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

Enable OIDC based Authentication with apisix #312

Merged
merged 9 commits into from
Sep 11, 2024

Conversation

ckhened
Copy link
Collaborator

@ckhened ckhened commented Aug 18, 2024

Description

The proposed changes enable OIDC (Open ID Connect) based user Authentication using APISIX API gateway and Keycloak Identity provider to OPEA apps.

This change introduces 2 helm charts:

  1. helm-charts/auth-apisix/apisix-helm: This helm chart installs APISIX api gateway and APISIX ingress controller
  2. helm-charts/auth-apisix/apis-crd-helm: This helm chart contains CRDs for authenticated APIs that needs to be published in API gateway

The Readme file in helm-charts/auth-apisix/README.md gives instructions to install keycloak, apisix and API CRDs

APISIX is apache licensed open source API gateway which is light weight, delivers high performance. It can work with docker or kubernetes and with any service mesh within kubernetes.

Issues

n/a

Type of change

List the type of change like below. Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • [X ] New feature (non-breaking change which adds new functionality)
  • Breaking change (fix or feature that would break existing design and interface)

Dependencies

  1. apisix api gateway
  2. apisix-ingress-controller
  3. keycloak

Tests

Verified locally on my test system

@ckhened ckhened requested a review from ftian1 August 18, 2024 01:20
@hshen14 hshen14 requested a review from lvliang-intel August 18, 2024 23:44
helm-charts/auth-apisix/README.md Outdated Show resolved Hide resolved
helm-charts/auth-apisix/apis-crd-helm/values.yaml Outdated Show resolved Hide resolved
@ckhened ckhened force-pushed the ckhened-apisix-authNZ branch 2 times, most recently from a37fc17 to 3bc5c7a Compare August 23, 2024 00:47
Copy link
Collaborator

@lianhao lianhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this could NOT be covered by CI test due to the pre-requests, maybe we could move this to the top level folder authN-authZ, because this also resolves authentication & authorization.

helm-charts/auth-apisix/README.md Outdated Show resolved Hide resolved
helm-charts/auth-apisix/README.md Outdated Show resolved Hide resolved
helm-charts/auth-apisix/README.md Outdated Show resolved Hide resolved
helm-charts/auth-apisix/values_apisix_gw.yaml Outdated Show resolved Hide resolved
helm-charts/auth-apisix/README.md Outdated Show resolved Hide resolved
@ckhened ckhened force-pushed the ckhened-apisix-authNZ branch 2 times, most recently from 485abc7 to 92be80d Compare August 23, 2024 23:30
@lianhao
Copy link
Collaborator

lianhao commented Aug 26, 2024

Also please fix the DCO error, thx

helm-charts/auth-apisix/README.md Outdated Show resolved Hide resolved
helm-charts/auth-apisix/README.md Outdated Show resolved Hide resolved
The access token, refresh token, userinfo and user roles can be obtained by invoking OIDC auth endpoint through UI or token endpoint through curl and providing user credentials. </br></br>

Below steps can be followed to get access token from keycloak and access the APISIX published ChatQnA API through curl

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please help explain your authentication and authorization scenarios here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree the proposal of the above folder layout.

@lianhao
Copy link
Collaborator

lianhao commented Aug 28, 2024

@ckhened PR #353 is merged. Please refactor based on that. Thanks!

@ckhened ckhened force-pushed the ckhened-apisix-authNZ branch from 3e1ec31 to c9f766c Compare August 29, 2024 00:58
Copy link
Collaborator

@lianhao lianhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove duplicated files under helm-charts directory. Also please add reference in file authN-authZ/README.md

@ckhened ckhened force-pushed the ckhened-apisix-authNZ branch from a6512e5 to 03083b6 Compare August 29, 2024 01:43
@ckhened
Copy link
Collaborator Author

ckhened commented Aug 29, 2024

Moved auth-apisix dir to authN-authZ, fixed CI checks and incorporated other comments.
I'll add authentication and authorization scenarios examples (comment from @Ruoyu-y ) in the next commit

authN-authZ/auth-apisix/README.md Show resolved Hide resolved
authN-authZ/auth-apisix/README.md Outdated Show resolved Hide resolved
@ckhened ckhened requested a review from lianhao September 11, 2024 07:13
@Ruoyu-y
Copy link
Collaborator

Ruoyu-y commented Sep 11, 2024

Moved auth-apisix dir to authN-authZ, fixed CI checks and incorporated other comments. I'll add authentication and authorization scenarios examples (comment from @Ruoyu-y ) in the next commit

This will not include in this PR, correct?

@mkbhanda mkbhanda merged commit ee907d6 into opea-project:main Sep 11, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants