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.

Sign up for GitHub

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

Jump to bottom

Authenticate Requests from PingSources #7320

Closed
creydr opened this issue Sep 29, 2023 · 13 comments · Fixed by #7525
Closed

Authenticate Requests from PingSources #7320

creydr opened this issue Sep 29, 2023 · 13 comments · Fixed by #7525
Assignees
@Zazzscoot @skyenam
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.

Comments

@creydr
Copy link
Member

creydr commented Sep 29, 2023 •
edited
Loading

As the Eventing OIDC feature track describes, sources must authenticate their requests. Therefor the PingSource must request a JWT and add it as a Bearer Token to its Authentication header.

When having #7175, we need to update the PingSource to add the Authentication header with a JWT to all outgoing requests.

In particular this means for the PingSource adapter:

  • when the sink has no audience defined:
    • no change in behavior
  • when the the sink has an audience defined:
    • Request a JWT via Provide a library for OIDC token management #7175
    • Add it as an http header to the cloudeventsSDK client via something like 
      headers := http.HeaderFrom(ctx)
headers.Set("Authentication", fmt.Sprintf("Bearer %s", jwt))
ctx = http.WithCustomHeader(ctx, headers)

client.Send(ctx, event)

Additional Information:
@creydr creydr added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Sep 29, 2023
@rahulii
Copy link
Contributor

rahulii commented Oct 2, 2023

/assign

@creydr
Copy link
Member Author

creydr commented Oct 20, 2023

As the required issues are done, this is ready to be worked on

@Cali0707
Copy link
Member

Hey @rahulii are you still planning on working on this?

@creydr
Copy link
Member Author

creydr commented Nov 10, 2023

@rahulii As we didn't hear anything on this since some time, I would unassign you from this to give other contributors a chance to work on this.
Feel free to assign it to you again, when you're ready to work on it.

@Zazzscoot
Copy link
Contributor

/assign

@skyenam
Copy link

skyenam commented Nov 12, 2023

/assign

@creydr
Copy link
Member Author

creydr commented Nov 17, 2023

Hello @Zazzscoot and @skyenam,
thanks for taking this. Are you working on this together? Do you have an ETA for a PR?

@Zazzscoot
Copy link
Contributor

Hey @creydr,
yes, we are working on this together. ETA for a PR should be around 2-3 weeks!

@creydr
Copy link
Member Author

creydr commented Nov 20, 2023

Hey @creydr, yes, we are working on this together. ETA for a PR should be around 2-3 weeks!

Awesome. Thanks for the update!

@mgencur mgencur mentioned this issue Dec 11, 2023
Closed
@creydr
Copy link
Member Author

creydr commented Dec 20, 2023

Hey @Zazzscoot and @skyenam,
@Leo6Leo finished his work in #7452 and did also some preparation in this PR, which should help you on your work too. I hope this helps :)

@Leo6Leo
Copy link
Member

Leo6Leo commented Dec 20, 2023

And feel free to ping me here or on CNCF slack if you guys need any help :) @Zazzscoot @skyenam

@creydr
Copy link
Member Author

creydr commented Dec 21, 2023

Hey @Zazzscoot and @skyenam,
are you still planning to work on this? Or do you need anything to get started on this?

@Zazzscoot
Copy link
Contributor

Hey @creydr
We were bogged down with setup, but I just figured it out and we are getting started now!

Zazzscoot added a commit to Zazzscoot/eventing that referenced this issue Dec 24, 2023
@Zazzscoot
knative#7320 WIP
17c775a
@Zazzscoot Zazzscoot mentioned this issue Dec 25, 2023
Merged
5 tasks
This was referenced Jan 5, 2024
Closed
Closed
knative-prow bot pushed a commit that referenced this issue Jan 18, 2024
@Zazzscoot @yijie-04
Authenticate requests from PingSources (#7525)
fb9be2b 
* #7320 WIP

* added missing address attributes to unit tests

* added serviceaccoutns/token to pingsource role

* added rule for service accounts (token)

* wrote e2e tests for pingsource OIDC

* removed dynamic role adding

* removed unnecessary packages

* defined tokenprovider, set audience/OIDCServiceAcc

* added check for source.Status.Auth

* testing for pr

* reran runner.go

* added oidc

* removed testing

* added oidc in pingsource test

* added audience nil check and specified destination in auth test

* added duckv1 to imports for test file

* ran goimports

* removed dupe ceode from mtping/runner.go

* ran goimportrs on test/auth/oidc_test.go

* fixing boilerplate for test/auth/features/oidc/pingsource.go

* removed format.sh

* removed unnecessary comments

---------

Co-authored-by: Yijie Wang <yijiewang0806@gmail.com>
knative-prow bot pushed a commit that referenced this issue Jan 30, 2024
@yijie-04 @Zazzscoot
Use filtered informer to watch OIDC service accounts (#7527)
ff52881 
* controller.go changed

* #7320 WIP

* WIP: Testing filtered informer (#7341)

* unit test passed

* Revert "Merge remote-tracking branch 'otherfork/main' into main"

This reverts commit 94cd51b, reversing
changes made to 0bf2982.

* Removed comments

* Changed to filtered informer for Subscription identity service account

* Changed to filtered informer for Sequence service accounts

* Changed to filtered informer for Parallel identity service accounts

* Changed to filtered informer for APIServerSource identity service account

* fixed unit tests

* added label selector for mtchannel_broker

* added filtered informer for sinkbinding identity service accounts

* added OIDC label selector in webhook

* added filtered informer for containersource  service accounts

* added filtered informer for pingsource service accounts

* added OIDC label selector in apiserver ctx

* added OIDC label selector in broker/filter

* added OIDC label selector in broker/ingress

* added OIDC label selector in in_memory/channel_dispatcher

* added OIDC label selector in mtping

* fixed unit test issues for pingsource

* fixed unit test for container source

* formatted files

* updated service account informer in apiserversource

* updated service account informers in other places

* small typo fix

* added actual value for OIDC label

* added a valid value for OIDClabelkey

* changed references of OIDCLabelKey

* fixed import path problem

* changed OIDCLabelSelector in all main.go files

* changed instances of OIDCLabelSelector in controller and controller test files

* deleted OIDC related labels from register.go

* fixed formatting issues

* Added value for OIDCLabelKey

---------

Co-authored-by: Scott <scottprotoss@gmail.com>
creydr pushed a commit to creydr/knative-eventing that referenced this issue Apr 24, 2024
@yijie-04 @Zazzscoot @creydr
Use filtered informer to watch OIDC service accounts (knative#7527)
519962f 
* controller.go changed

* knative#7320 WIP

* WIP: Testing filtered informer (knative#7341)

* unit test passed

* Revert "Merge remote-tracking branch 'otherfork/main' into main"

This reverts commit 94cd51b, reversing
changes made to 0bf2982.

* Removed comments

* Changed to filtered informer for Subscription identity service account

* Changed to filtered informer for Sequence service accounts

* Changed to filtered informer for Parallel identity service accounts

* Changed to filtered informer for APIServerSource identity service account

* fixed unit tests

* added label selector for mtchannel_broker

* added filtered informer for sinkbinding identity service accounts

* added OIDC label selector in webhook

* added filtered informer for containersource  service accounts

* added filtered informer for pingsource service accounts

* added OIDC label selector in apiserver ctx

* added OIDC label selector in broker/filter

* added OIDC label selector in broker/ingress

* added OIDC label selector in in_memory/channel_dispatcher

* added OIDC label selector in mtping

* fixed unit test issues for pingsource

* fixed unit test for container source

* formatted files

* updated service account informer in apiserversource

* updated service account informers in other places

* small typo fix

* added actual value for OIDC label

* added a valid value for OIDClabelkey

* changed references of OIDCLabelKey

* fixed import path problem

* changed OIDCLabelSelector in all main.go files

* changed instances of OIDCLabelSelector in controller and controller test files

* deleted OIDC related labels from register.go

* fixed formatting issues

* Added value for OIDCLabelKey

---------

Co-authored-by: Scott <scottprotoss@gmail.com>
openshift-merge-bot bot pushed a commit to openshift-knative/eventing that referenced this issue Apr 24, 2024
@creydr @yijie-04 @Zazzscoot
[release-v1.12] Reconcile trigger on OIDC service account changes onl…
bbb4c41 
…y, if SA references a trigger for correct broker class (#592)

* Use filtered informer to watch OIDC service accounts (knative#7527)

* controller.go changed

* knative#7320 WIP

* WIP: Testing filtered informer (knative#7341)

* unit test passed

* Revert "Merge remote-tracking branch 'otherfork/main' into main"

This reverts commit 94cd51b, reversing
changes made to 0bf2982.

* Removed comments

* Changed to filtered informer for Subscription identity service account

* Changed to filtered informer for Sequence service accounts

* Changed to filtered informer for Parallel identity service accounts

* Changed to filtered informer for APIServerSource identity service account

* fixed unit tests

* added label selector for mtchannel_broker

* added filtered informer for sinkbinding identity service accounts

* added OIDC label selector in webhook

* added filtered informer for containersource  service accounts

* added filtered informer for pingsource service accounts

* added OIDC label selector in apiserver ctx

* added OIDC label selector in broker/filter

* added OIDC label selector in broker/ingress

* added OIDC label selector in in_memory/channel_dispatcher

* added OIDC label selector in mtping

* fixed unit test issues for pingsource

* fixed unit test for container source

* formatted files

* updated service account informer in apiserversource

* updated service account informers in other places

* small typo fix

* added actual value for OIDC label

* added a valid value for OIDClabelkey

* changed references of OIDCLabelKey

* fixed import path problem

* changed OIDCLabelSelector in all main.go files

* changed instances of OIDCLabelSelector in controller and controller test files

* deleted OIDC related labels from register.go

* fixed formatting issues

* Added value for OIDCLabelKey

---------

Co-authored-by: Scott <scottprotoss@gmail.com>

* Reconcile trigger on OIDC service account changes only, if SA references a trigger for correct broker class (knative#7849)

* Reconcile trigger on OIDC service account changes only, if SA references a trigger for correct broker class

* Run goimports and gofmt

* Remove deprecated use of pointer.Bool(v) and switch to prt.Bool(v)

---------

Co-authored-by: Yijie Wang <147119743+yijie-04@users.noreply.github.com>
Co-authored-by: Scott <scottprotoss@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Zazzscoot Zazzscoot

@skyenam skyenam

Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.
Projects
Eventing Sender Identity
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Authenticate requests from PingSources Zazzscoot/eventing
6 participants
@creydr @rahulii @Leo6Leo @Cali0707 @Zazzscoot @skyenam