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

Update appeals_api rake tasks for new OAuth scopes #12368

Merged
merged 0 commits into from
Apr 14, 2023
Merged

Update appeals_api rake tasks for new OAuth scopes #12368

merged 0 commits into from
Apr 14, 2023

Conversation

caseywilliams
Copy link
Contributor

@caseywilliams caseywilliams commented Apr 10, 2023 •
edited
Loading

Summary

  • Update appeals rake tasks for OAuth tokens so that:
    • Newly defined scopes are used instead of old ones which will be deleted soon (see API-25677 - New appeals scopes #12364 for more on these new scopes)
    • There are new tasks that can be used to fetch authorization code flow tokens (i.e. not-CCG, representing an individual user). This is a simple docker container invocation, but the parameters to pass are not totally straightforward, so I've wrapped the invocation in a rake task here - I had to refactor a bit to accommodate this, hence the larger number of lines changed here
      - This PR is separate from API-25677 - New appeals scopes #12364 because of the PR check on changed lines, but it depends on those changes - this should not be merged (and won't work correctly) until API-25677 - New appeals scopes #12364 is merged and I've rebased this PR. Done

Related issue(s)

Testing done

  • Requested some tokens with the new CCG and auth code flow scopes locally
  • Validated them locally with validation tasks
  • Made a few successful requests to local endpoints with these new tokens

What areas of the site does it impact?

  • Just the oauth rake tasks for the appeals APIs

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog or Grafana (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

@caseywilliams caseywilliams added Lighthouse lighthouse appeals Lighthouse API appeals banana-peels Lighthouse Banana Peels Team labels Apr 10, 2023
@caseywilliams caseywilliams mentioned this pull request Apr 10, 2023
Merged
8 tasks
@caseywilliams caseywilliams added the NOT_YET Don't merge this PR w/o the authors permission label Apr 10, 2023
@va-vfs-bot va-vfs-bot temporarily deployed to API-25677/updated-rake-task-appeals-scopes/main/main April 10, 2023 18:46 Inactive
caseywilliams
caseywilliams commented Apr 10, 2023
View reviewed changes
config/settings.yml
@@ -284,14 +284,22 @@ modules_appeals_api:
api_key: ''
# The token generation values below are only used for development rake tasks.
token_generation:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These settings values are only used in local development workflows, so there's nothing related to update in the deployed environments

@caseywilliams caseywilliams force-pushed the API-25677/updated-rake-task-appeals-scopes branch from cc57b66 to 040dd81 Compare April 11, 2023 00:02
@va-vsp-bot va-vsp-bot requested a deployment to master/main/API-25677/updated-rake-task-appeals-scopes April 11, 2023 00:03 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-25677/updated-rake-task-appeals-scopes/main/main April 11, 2023 00:04 Inactive
@caseywilliams caseywilliams force-pushed the API-25677/updated-rake-task-appeals-scopes branch from 040dd81 to c0c749a Compare April 11, 2023 23:11
@va-vsp-bot va-vsp-bot requested a deployment to master/main/API-25677/updated-rake-task-appeals-scopes April 11, 2023 23:12 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-25677/updated-rake-task-appeals-scopes/main/main April 11, 2023 23:12 Inactive
@caseywilliams caseywilliams force-pushed the API-25677/updated-rake-task-appeals-scopes branch from c0c749a to 90a41ec Compare April 13, 2023 17:53
@caseywilliams caseywilliams removed the NOT_YET Don't merge this PR w/o the authors permission label Apr 13, 2023
@caseywilliams caseywilliams marked this pull request as ready for review April 13, 2023 17:53
@caseywilliams caseywilliams requested review from a team as code owners April 13, 2023 17:53
@va-vsp-bot va-vsp-bot requested a deployment to master/main/API-25677/updated-rake-task-appeals-scopes April 13, 2023 17:53 In progress
caseywilliams
caseywilliams commented Apr 13, 2023
View reviewed changes
config/settings.yml
auth:
config_uri: 'https://dev-api.va.gov/oauth2/appeals/v1/.well-known/openid-configuration'
# Get a client ID/secret by submitting this form - make sure to check the boxes for the APIs under
# "Authorization Code Flow APIs": https://developer.va.gov/onboarding/request-sandbox-access
Copy link
Contributor Author

@caseywilliams caseywilliams Apr 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

N.B. for now, you can use https://dev-developer.va.gov/onboarding/request-sandbox-access to get tokens instead, since these APIs are only live in dev

@va-vfs-bot va-vfs-bot temporarily deployed to API-25677/updated-rake-task-appeals-scopes/main/main April 13, 2023 17:59 Inactive
mathisto
mathisto reviewed Apr 13, 2023
View reviewed changes
rakelib/appeals_api_oauth.rake Outdated
API_NAMES.each do |api_name|
namespace abbreviate_snake_case_name(api_name).to_sym do
AppealsRakeHelpers::API_NAMES.each do |api_name|
namespace api_name.scan(/(?<=^|_)(\S)/).join.to_sym do
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider keeping a wrapper for this regex. This seems more difficult to parse at a glance than the existing method name.

mathisto
mathisto previously approved these changes Apr 13, 2023
View reviewed changes
Copy link
Contributor

@mathisto mathisto left a comment

Choose a reason for hiding this comment

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

Gold star 🌟 My comment on the regex is not a blocker.

kristen-brown
kristen-brown previously approved these changes Apr 13, 2023
View reviewed changes
@caseywilliams caseywilliams dismissed stale reviews from kristen-brown and mathisto via e957f09 April 13, 2023 22:18
@va-vsp-bot va-vsp-bot requested a deployment to master/main/API-25677/updated-rake-task-appeals-scopes April 13, 2023 22:19 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-25677/updated-rake-task-appeals-scopes/main/main April 13, 2023 22:27 Inactive
@caseywilliams caseywilliams force-pushed the API-25677/updated-rake-task-appeals-scopes branch from e957f09 to 3f827ac Compare April 13, 2023 22:43
@va-vsp-bot va-vsp-bot requested a deployment to master/main/API-25677/updated-rake-task-appeals-scopes April 13, 2023 22:44 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-25677/updated-rake-task-appeals-scopes/main/main April 13, 2023 22:47 Inactive
mathisto
mathisto reviewed Apr 14, 2023
View reviewed changes
rakelib/appeals_api_oauth.rake

def abbreviate_snake_case_name(name)
name.scan(/(?<=^|_)(\S)/).join
def abbreviate_snake_case_name(name) = name.scan(/(?<=^|_)(\S)/).join
Copy link
Contributor

Choose a reason for hiding this comment

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

Noice! Endless def!

@caseywilliams caseywilliams force-pushed the API-25677/updated-rake-task-appeals-scopes branch from 3f827ac to 3ec2573 Compare April 14, 2023 15:27
@va-vsp-bot va-vsp-bot requested a deployment to master/main/API-25677/updated-rake-task-appeals-scopes April 14, 2023 15:28 In progress
@caseywilliams caseywilliams merged commit 5e8df63 into master Apr 14, 2023
@caseywilliams caseywilliams deleted the API-25677/updated-rake-task-appeals-scopes branch April 14, 2023 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kristen-brown kristen-brown kristen-brown approved these changes

@mathisto mathisto mathisto approved these changes

Assignees
No one assigned
Labels
appeals Lighthouse API appeals banana-peels Lighthouse Banana Peels Team console-services-review Lighthouse lighthouse
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@caseywilliams @mathisto @kristen-brown @va-vfs-bot @va-vsp-bot