InMemoryChannel ingress: added getOIDC #8104

7h3-3mp7y-m4n · 2024-07-17T19:45:10Z

Proposed Changes

added "Get the OIDC identity of the sender"

Pre-review Checklist

At least 80% unit test coverage
E2E tests for any new behavior
Docs PR for any user-facing impact
Spec PR for any new API feature
Conformance test for any change to the spec

knative-prow · 2024-07-17T19:45:20Z

Hi @7h3-3mp7y-m4n. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

knative-prow · 2024-07-17T19:45:23Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 7h3-3mp7y-m4n
Once this PR has been reviewed and has the lgtm label, please assign skonto for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

7h3-3mp7y-m4n · 2024-07-17T19:46:46Z

I'm working on the further tasks, please let me know if I implemented the wrong approach.

Leo6Leo · 2024-07-17T20:01:51Z

Thank you for your PR! @7h3-3mp7y-m4n
Please don't hesitate reaching out if you have any questions while working on the issue.

/cc @rahulii

rahulii · 2024-07-19T08:02:59Z

pkg/channel/event_receiver.go

@@ -256,12 +256,13 @@ func (r *EventReceiver) ServeHTTP(response nethttp.ResponseWriter, request *neth
 	features := feature.FromContext(ctx)
 	if features.IsOIDCAuthentication() {
 		r.logger.Debug("OIDC authentication is enabled")
-		err = r.tokenVerifier.VerifyJWTFromRequest(ctx, request, &r.audience, response)
+		oidcToken, err := r.tokenVerifier.GetOIDCIdentity(ctx, request, r.audience)


The goal here is to verify the the request, I'd not change the existing code instead use a seperate function to get the OIDC Token!

Okay , I'll add a separate function to do that and leave it as it was ..
Thanks !

rahulii · 2024-07-19T08:04:21Z

pkg/auth/token_verifier.go

@@ -179,3 +179,19 @@ type openIDMetadata struct {
 	SubjectTypes  []string `json:"subject_types_supported"`
 	SigningAlgs   []string `json:"id_token_signing_alg_values_supported"`
 }
+
+// Getting the OIDCIdentity
+func (c *OIDCTokenVerifier) GetOIDCIdentity(ctx context.Context, r *http.Request, audience string) (*IDToken, error) {


As mentioned in the other comment, the goal if your function is to get OIDC Identity and not verify the request as you are doing on line 191.

Leo6Leo · 2024-07-19T12:17:26Z

Thanks for reaching out for help @7h3-3mp7y-m4n , I'm replying on GitHub to make our conversation more open!

Let's walk through this step by step. Let me provide more explaination on that description first.

Check if policies are set:
First, the system looks at the .status.policies field of the InMemoryChannel.
If policies are set (.status.policies is not empty):
- The system checks if the sender's identity is listed in the .status.from[] of any of the linked EventPolicies.
- If the sender's identity is found: The request is allowed to continue.
- If the sender's identity is not found: The request is rejected with a 403 (Forbidden) status code.
If no policies are set (.status.policies is empty, which means there is no specified policies applied, we will need to use the default setting)):
The system then checks the default-authorization-mode setting, which can have three possible values:

a. allow-all:
- All requests are allowed to continue, regardless of the sender's identity.
b. deny-all:
- All requests are rejected with a 403 (Forbidden) status code.
c. allow-same-namespace:
- The system checks if the sender's identity is from the same namespace as the InMemoryChannel.
- If it's from the same namespace: The request is allowed to continue.
- If it's from a different namespace: The request is rejected with a 403 status code.

This logic provides a flexible authorization mechanism:

When specific policies are set, it allows fine-grained control over which identities can send events.
When no policies are set, it falls back to a configurable default behavior, which can be permissive, restrictive, or namespace-based.

Leo6Leo · 2024-07-19T12:20:16Z

Next, regarding your question:

how to retrieve the info for event policy then i would be a great help
from my end I'm thinking to invoke a function which checks that sender has some policy or not and if not then passing the http error codes .

In this PR, @rahulii has tried to put the event policies into .status.policies.

If you want to list all the eventPolicies, take a look here:
https://github.com/knative/eventing/pull/8090/files#diff-076a85d02ed58938b812d9a1f6fde6d842063744420d38c66448793de5e8ddb4R475

Lmk if you have any further questions, and I hope this help.

cc @rahulii

codecov · 2024-07-19T13:01:08Z

Codecov Report

Attention: Patch coverage is 9.52381% with 38 lines in your changes missing coverage. Please review.

Project coverage is 67.71%. Comparing base (3ee2400) to head (dfa5e4b).
Report is 69 commits behind head on main.

Files	Patch %	Lines
pkg/auth/token_verifier.go	0.00%	29 Missing ⚠️
pkg/channel/event_receiver.go	30.76%	8 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8104      +/-   ##
==========================================
- Coverage   69.30%   67.71%   -1.59%     
==========================================
  Files         345      368      +23     
  Lines       16113    17597    +1484     
==========================================
+ Hits        11167    11916     +749     
- Misses       4257     4935     +678     
- Partials      689      746      +57

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

7h3-3mp7y-m4n · 2024-07-19T19:58:44Z

Hey @Leo6Leo can you guide me to the InmemoryChannel Code, it would be helpful to retrieve the policies and check according to that, and also a few more details about the default-authorization-mode

rahulii · 2024-07-22T06:40:05Z

Hey @Leo6Leo can you guide me to the InmemoryChannel Code, it would be helpful to retrieve the policies and check according to that, and also a few more details about the default-authorization-mode

hi @7h3-3mp7y-m4n if you want to read more about the feature - please read Knative Eventing Authorization

rahulii · 2024-07-31T12:42:11Z

hey @7h3-3mp7y-m4n , do you need any help here? or Please LMK if you plan to complete this PR, if you are busy, I can take this up if you want.
Thanks!
cc: @Leo6Leo

7h3-3mp7y-m4n · 2024-07-31T12:48:41Z

hey @7h3-3mp7y-m4n , do you need any help here? or Please LMK if you plan to complete this PR, if you are busy, I can take this up if you want. Thanks! cc: @Leo6Leo

Thanks @rahulii I read the documentation of the feature as you provided it to me, I'll get this done today itself, No worries.

creydr · 2024-08-12T06:50:23Z

Closing this in favor of #8123

InMemoryChannel ingress: added getOIDC

91d4b4a

knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 17, 2024

knative-prow bot requested review from matzew and pierDipi July 17, 2024 19:45

knative-prow bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 17, 2024

knative-prow bot requested a review from rahulii July 17, 2024 20:01

rahulii reviewed Jul 19, 2024

View reviewed changes

InMemoryChannel ingress: Added the desire changes

dfa5e4b

knative-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 19, 2024

creydr closed this Aug 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

InMemoryChannel ingress: added getOIDC #8104

InMemoryChannel ingress: added getOIDC #8104

7h3-3mp7y-m4n commented Jul 17, 2024

knative-prow bot commented Jul 17, 2024

knative-prow bot commented Jul 17, 2024

7h3-3mp7y-m4n commented Jul 17, 2024

Leo6Leo commented Jul 17, 2024

rahulii Jul 19, 2024

7h3-3mp7y-m4n Jul 19, 2024

rahulii Jul 19, 2024

Leo6Leo commented Jul 19, 2024

Leo6Leo commented Jul 19, 2024

codecov bot commented Jul 19, 2024

7h3-3mp7y-m4n commented Jul 19, 2024

rahulii commented Jul 22, 2024

rahulii commented Jul 31, 2024

7h3-3mp7y-m4n commented Jul 31, 2024

creydr commented Aug 12, 2024

InMemoryChannel ingress: added getOIDC #8104

InMemoryChannel ingress: added getOIDC #8104

Conversation

7h3-3mp7y-m4n commented Jul 17, 2024

Proposed Changes

Pre-review Checklist

knative-prow bot commented Jul 17, 2024

knative-prow bot commented Jul 17, 2024

7h3-3mp7y-m4n commented Jul 17, 2024

Leo6Leo commented Jul 17, 2024

rahulii Jul 19, 2024

Choose a reason for hiding this comment

7h3-3mp7y-m4n Jul 19, 2024

Choose a reason for hiding this comment

rahulii Jul 19, 2024

Choose a reason for hiding this comment

Leo6Leo commented Jul 19, 2024

Leo6Leo commented Jul 19, 2024

codecov bot commented Jul 19, 2024

Codecov Report

7h3-3mp7y-m4n commented Jul 19, 2024

rahulii commented Jul 22, 2024

rahulii commented Jul 31, 2024

7h3-3mp7y-m4n commented Jul 31, 2024

creydr commented Aug 12, 2024