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

14601 authorization api #16495

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

14601 authorization api #16495

wants to merge 25 commits into from

Conversation

jalbinson
Copy link
Collaborator

@jalbinson jalbinson commented Nov 6, 2024

This PR adds authorization functionality to the auth and submission microservices.

Test Steps:

  1. Get secrets and active access token from Jamie
  2. Start up ReportStream, submissions, and auth (look at document in this PR for steps)
  3. Submit a report to http://localhost:9000/api/v1/reports
  4. Ensure the response you get is NOT 401 or 403. If something goes wrong in submissions thats beyond the scope of this ticket.

Bonus points: run the tests in the auth and submissions project to ensure they are passing on your machine locally.

Changes

  • Okta admin API calls to retrieve group information
  • Added Okta-Groups header with a JWT value
  • JWT read/writing
  • Authorization logic for senders

Checklist

Testing

  • Tested locally?
  • Ran ./prime test or ./gradlew testSmoke against local Docker ReportStream container?
  • Added tests?

Linked Issues

@jalbinson jalbinson changed the title Platform/jamie/14601 authz api 14601 authz api Nov 7, 2024
@jalbinson jalbinson changed the title 14601 authz api 14601 authorization api Nov 7, 2024
@jalbinson jalbinson marked this pull request as ready for review November 7, 2024 21:27
@jalbinson jalbinson added the platform Platform Team label Nov 7, 2024
@MichaelEsuruoso MichaelEsuruoso assigned JFisk42 and adegolier and unassigned JFisk42 Nov 8, 2024
@jalbinson jalbinson added the microservice Tickets that are required to properly support the microservice arch label Nov 8, 2024

```bash
# from project root
# start report stream and all dependant docker containers
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# start report stream and all dependant docker containers
# start ReportStream and all dependent docker containers


## Submitting reports locally

- Retrieve an access token directly from Okta and ensure the JWT contains the "sender" scope
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please include instructions on how to do this here

Copy link
Collaborator

@adegolier adegolier left a comment

Choose a reason for hiding this comment

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

Tried to follow the test steps, but I get connection refused when I try to submit a report.

for scripts to run to generate new keys.

By Default, we are connecting to the Staging Okta. We cannot post connection secrets directly in this document so
you will have to ask someone for those values.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe instructions for retrieving the values if you have the appropriate Okta access? And/or a note that you have to be an Okta admin (not a RS admin) to access that information.

Side note: Does that mean that onboarding folks will all have to be Okta admins to be able to set up new users?

.setClientId(oktaClientProperties.clientId)
.setScopes(oktaClientProperties.requiredScopes)
.setPrivateKey(oktaClientProperties.apiPrivateKey)
// .setCacheManager(...) TODO: investigate caching since groups don't often change
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a ticket for this?

nbf: 5s
ttl: 5m
issuer: http://localhost:9000
jwtEncodedPrivateKeyJWK: eyJwIjoieC1yaFlLVVM2TnpRd0FCYjVmakNEMGl1NlM0MFAySUtQd3pnaUJqU3VjTDZ3VFpoYU4tbTBfZHBEdGhwMHZVQjQyR01LQ2ZHNklWNlFHSV91eF80c3I1QTBRSF9sY09jNC1DbTBWal9Udy1DcFRYbnRaVlJSSDlSN00tdDJ1WXNvT1ZyaXJsUUwxdW1ybThPdjkzVnBUWnJ4QlB6UXBlejZvVGJxZUQ0WF9jIiwia3R5IjoiUlNBIiwicSI6InlYYlhQamJHRmo0cUN3RWJqSUpMQ3hQS1FkTnduX2MtQ0M0ZVBma3UyNTR2VW1jeE1IQ1Q0UGtmQ3ZiTXlKU2plWUxzRFZwY0M1eThqeGJwLXpVMUF3WDJvZk15ZEhJa0RfSW51cjVXM1J2U1ZGUFFqR01YVk5Qc09LU1hxRUUzb3BfSU9QZEtUcXhBR3NZMkhYVEZDYVJSU25SdHMwQnV4ZS10SlZjbnpHcyIsImQiOiJQdmk3YnpOZy1ZekZQcWxsRlNHdXk0UnV5cGE0SDctN1lIZnZEb1pKVktqZFZIcHBqVHNsbjRmZWxicEYxQWJ1c2R2blJnU2QzSHVWQWVTTi1rT2dRbDR6SlNJLThvdDhSd29EQU4wSXNadGg0UDhSbTMwa0JFcFdMR0xPb3Q3ZkFWRDRMSzJFTjVEUlU5dWVVWmNoaFpmcDJwaGl2cHJ4TVluTnlhMzNXSm1oTHdSUFRRVjBIY3JmYk9kVG82a2FHUllfZkpCdHRjd2QzWlQ0dzljOXd6ZzFJT0t6Ul96S2g5VkR1b2Y3SWF0ekNab0ZzeUZ5ZHgwbHZxalZjRXAxSllQMTFBY0VOaUdYOHBSTXpzVktmaUhuNlBET2ZoSXBTU1lBd21wYWI3WGF6V2ZWbEZyam5VQXlpN2wyZV90U1lwMGc1RnpZcjB6NDZrTWo1YnBsIiwiZSI6IkFRQUIiLCJxaSI6IkdWbTR6WllDeTNVNEF6TmhSdE5FWFhzVl9ZYm9CZXR5SS1FRjhhcHNsUGc4YTBhMElzXzFZTTZndTQ4ZlNndVE1WFRpN2RVMURJLTZaR2JSbmVvMXVGb0R1M2RsVGR1dXdrcWx6XzJIREphY1dWZjFWQkRSNkdBSjJ3eWNXckdMM0VkM0k4eGtBeHUzMnlYUGJ0YjlpUy1pRF9WQmo1LVJoSktIUXhHbGNSTSIsImRwIjoid0p5V1JHMEd5UUJteDNZUkZJTVZSWEI3eFFIVktQUW1keFRMQjVVVEFoTFBVWFE1YWJlQm5sdVRCdENQTk1jRjZMTkZQRE1HdTJST290V0dIWjN5R1JTZ2tqN2dwc1J1MWtiTnNvbVNnZk9wcGM5SHpYVnRkUmRPTVdEdVdpYkZfTWJOVkR5eS1zM015LWNJU09kTVBmOHUyUjEzbEVOZ19xUy1sdS1fbllVIiwiZHEiOiJyRXd2MzJ4VzB4VU5QZVlQbW9hZ0NYUS1hVGVjdmFKazhmZ0hNemRXUk1zdmE0a0hmNGI0WWRLTkl3Slp0ejJ2NWE3N2xKdnYxcHFRaE11ekJuM0Z2YlV1N2VpaEFRZlJJYllYRmxYTTBrTUdDY3E0dENmV18xeFRUVW91emQ0ZzU3dEJNTDhGVk8xcDBid3M4ZG80M1hzamJzck9PeHhpNEhPUG9EeS1zOHMiLCJuIjoiblZRNVQ1MFk3Q3NBZkhfWllzQjBhRVFHYnlYcklQdDU1dG5OTGZNSWQwbWJYM1ljdVB6cVVQbVJzYlhiNmNPYmlXT1k1Znk2UGxfaEZDeUs2em9qU1JxaFdCd2dMcHo2NUg3NVJ2bVk5Y2FQbndLSXdPOWhpS2ZHTW5DMkdvS3U2S1otcFFLNXlHaUUwRGhVRGE0Q3gxa0h4NlJUUzVoSXlHZHRFOXk5eGlCU0RJODI0eXVwZ1Z5SkYtU1cteHBINVFYdy1saUxGdVBwSFpnWnQyNEhVRlBPM1JqcXJXTmNkWUZ0S00tZGhzWkxMQXp0cEZSMGlqY3M1SEdjbUVlWnRpaVBJYzFpUF9NWlZGSV9SbnVQdzBxbENFRDF4UGdqeXhzT3ZScDlIdU85NDVfenlib2tyYzlVbTljZnpTTEd3WmdJSW9HZVdtc2VRYWR0Zy1ud1BRIn0=
Copy link
Collaborator

Choose a reason for hiding this comment

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

Private key in the code base seems.... not good? I'm not super clear on what this setup is for, but it looks like it's pointing at the Staging Okta instance?

@Value("\${wiremock.server.port}") val port: Int,
) {

// we have to set this up this way rather than using @TestMock because of kotlin coroutine support
Copy link
Collaborator

Choose a reason for hiding this comment

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

Appreciate the comments here about why we're doing it this way instead of another way ❤️

path: /swagger/api-docs

#Uncomment for verbose logging
Copy link
Collaborator

Choose a reason for hiding this comment

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

This appears to be uncommented already, is that intentional?

jwtEncodedPrivateKeyJWK: eyJwIjoieC1yaFlLVVM2TnpRd0FCYjVmakNEMGl1NlM0MFAySUtQd3pnaUJqU3VjTDZ3VFpoYU4tbTBfZHBEdGhwMHZVQjQyR01LQ2ZHNklWNlFHSV91eF80c3I1QTBRSF9sY09jNC1DbTBWal9Udy1DcFRYbnRaVlJSSDlSN00tdDJ1WXNvT1ZyaXJsUUwxdW1ybThPdjkzVnBUWnJ4QlB6UXBlejZvVGJxZUQ0WF9jIiwia3R5IjoiUlNBIiwicSI6InlYYlhQamJHRmo0cUN3RWJqSUpMQ3hQS1FkTnduX2MtQ0M0ZVBma3UyNTR2VW1jeE1IQ1Q0UGtmQ3ZiTXlKU2plWUxzRFZwY0M1eThqeGJwLXpVMUF3WDJvZk15ZEhJa0RfSW51cjVXM1J2U1ZGUFFqR01YVk5Qc09LU1hxRUUzb3BfSU9QZEtUcXhBR3NZMkhYVEZDYVJSU25SdHMwQnV4ZS10SlZjbnpHcyIsImQiOiJQdmk3YnpOZy1ZekZQcWxsRlNHdXk0UnV5cGE0SDctN1lIZnZEb1pKVktqZFZIcHBqVHNsbjRmZWxicEYxQWJ1c2R2blJnU2QzSHVWQWVTTi1rT2dRbDR6SlNJLThvdDhSd29EQU4wSXNadGg0UDhSbTMwa0JFcFdMR0xPb3Q3ZkFWRDRMSzJFTjVEUlU5dWVVWmNoaFpmcDJwaGl2cHJ4TVluTnlhMzNXSm1oTHdSUFRRVjBIY3JmYk9kVG82a2FHUllfZkpCdHRjd2QzWlQ0dzljOXd6ZzFJT0t6Ul96S2g5VkR1b2Y3SWF0ekNab0ZzeUZ5ZHgwbHZxalZjRXAxSllQMTFBY0VOaUdYOHBSTXpzVktmaUhuNlBET2ZoSXBTU1lBd21wYWI3WGF6V2ZWbEZyam5VQXlpN2wyZV90U1lwMGc1RnpZcjB6NDZrTWo1YnBsIiwiZSI6IkFRQUIiLCJxaSI6IkdWbTR6WllDeTNVNEF6TmhSdE5FWFhzVl9ZYm9CZXR5SS1FRjhhcHNsUGc4YTBhMElzXzFZTTZndTQ4ZlNndVE1WFRpN2RVMURJLTZaR2JSbmVvMXVGb0R1M2RsVGR1dXdrcWx6XzJIREphY1dWZjFWQkRSNkdBSjJ3eWNXckdMM0VkM0k4eGtBeHUzMnlYUGJ0YjlpUy1pRF9WQmo1LVJoSktIUXhHbGNSTSIsImRwIjoid0p5V1JHMEd5UUJteDNZUkZJTVZSWEI3eFFIVktQUW1keFRMQjVVVEFoTFBVWFE1YWJlQm5sdVRCdENQTk1jRjZMTkZQRE1HdTJST290V0dIWjN5R1JTZ2tqN2dwc1J1MWtiTnNvbVNnZk9wcGM5SHpYVnRkUmRPTVdEdVdpYkZfTWJOVkR5eS1zM015LWNJU09kTVBmOHUyUjEzbEVOZ19xUy1sdS1fbllVIiwiZHEiOiJyRXd2MzJ4VzB4VU5QZVlQbW9hZ0NYUS1hVGVjdmFKazhmZ0hNemRXUk1zdmE0a0hmNGI0WWRLTkl3Slp0ejJ2NWE3N2xKdnYxcHFRaE11ekJuM0Z2YlV1N2VpaEFRZlJJYllYRmxYTTBrTUdDY3E0dENmV18xeFRUVW91emQ0ZzU3dEJNTDhGVk8xcDBid3M4ZG80M1hzamJzck9PeHhpNEhPUG9EeS1zOHMiLCJuIjoiblZRNVQ1MFk3Q3NBZkhfWllzQjBhRVFHYnlYcklQdDU1dG5OTGZNSWQwbWJYM1ljdVB6cVVQbVJzYlhiNmNPYmlXT1k1Znk2UGxfaEZDeUs2em9qU1JxaFdCd2dMcHo2NUg3NVJ2bVk5Y2FQbndLSXdPOWhpS2ZHTW5DMkdvS3U2S1otcFFLNXlHaUUwRGhVRGE0Q3gxa0h4NlJUUzVoSXlHZHRFOXk5eGlCU0RJODI0eXVwZ1Z5SkYtU1cteHBINVFYdy1saUxGdVBwSFpnWnQyNEhVRlBPM1JqcXJXTmNkWUZ0S00tZGhzWkxMQXp0cEZSMGlqY3M1SEdjbUVlWnRpaVBJYzFpUF9NWlZGSV9SbnVQdzBxbENFRDF4UGdqeXhzT3ZScDlIdU85NDVfenlib2tyYzlVbTljZnpTTEd3WmdJSW9HZVdtc2VRYWR0Zy1ud1BRIn0=

# Ensure these are disabled in production
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm confused by this comment. I didn't think test resources would be part of the build for production, and if these should be disabled, shouldn't they be so before merging?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should probably remove commented out code to prevent confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
microservice Tickets that are required to properly support the microservice arch platform Platform Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement ReportStream AUTH-Z API
5 participants