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

Add and wire up swagger file for admin API #1980

Merged
merged 17 commits into from
May 10, 2019

Conversation

reggieriser
Copy link
Contributor

@reggieriser reggieriser commented Apr 8, 2019

Description

This PR adds and wires up a swagger file for the admin API.

Reviewer Notes

Note that I have a placeholder endpoint with no implementation currently in the admin.yaml swagger file. Without any endpoints, go-swagger will throw an error. This will be removed as we add actual admin endpoints.

Setup

  1. make server_run
  2. make admin_client_run
  3. Try the following local URLs:

There's also a login page in place for admin right now like our other apps, found at http://adminlocal:3000/ . This login process may change some once we wire in auth for React Admin, but having the login allows us to test login.gov integration and other wiring that goes along with that. Note that in development right now we're getting back different UUIDs (which we store in the users table) from login.gov for each of our apps, so you will have to do some database manipulation to test a successful login:

  1. Try to login to admin. You should get an Unauthorized response after authentication. This is because there's no users record with the UUID that came back from login.gov.
  2. In authentication/auth.go, either set a breakpoint on the call to FetchUserIdentity and examine the UserID UUID there, or turn on pop debugging around that call to see the SQL locally. Note the UUID that it's looking for.
  3. Create a users record with login_gov_uuid set to the UUID noted in step 2. Use your login.gov email for the login_gov_email column. Make sure you set is_superuser to true. The id for the record can be any UUID you generate.
  4. Try logging in again -- you should get a page saying React admin is properly configured.

We plan to address the multiple UUIDs from login.gov in a later story -- it may affect more than just the admin app.

Code Review Verification Steps

  • Code follows the guidelines for Logging
  • This works in Supported Browsers (Chrome, Firefox, IE, Edge).
  • Tested in the Experimental environment (for changes to containers, app startup, or connection to data stores)
  • Request review from a member of a different team.
  • Have the Pivotal acceptance criteria been met for this change?

References

@codecov
Copy link

codecov bot commented Apr 8, 2019

Codecov Report

Merging #1980 into master will decrease coverage by 0.12%.
The diff coverage is 28.07%.

@@            Coverage Diff             @@
##           master    #1980      +/-   ##
==========================================
- Coverage   60.33%   60.22%   -0.12%     
==========================================
  Files         231      231              
  Lines       13392    13445      +53     
==========================================
+ Hits         8080     8096      +16     
- Misses       4337     4370      +33     
- Partials      975      979       +4

Copy link
Contributor

@chrisgilmerproj chrisgilmerproj left a comment

Choose a reason for hiding this comment

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

Putting a hold on this until we get the cert up.

Copy link
Contributor

@chrisgilmerproj chrisgilmerproj left a comment

Choose a reason for hiding this comment

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

Before you can merge we need to set up the other apps in the sandbox. If we don't then this might throw errors in the different environments if people try to log in to the admin. Grab me tomorrow and we'll get that done.

Copy link
Contributor

@chrisgilmerproj chrisgilmerproj left a comment

Choose a reason for hiding this comment

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

🚀 - Nice work!

@reggieriser reggieriser changed the title [WIP] Add and wire up swagger file for admin API Add and wire up swagger file for admin API May 10, 2019
@reggieriser reggieriser removed the wip label May 10, 2019
@LeDeep
Copy link
Contributor

LeDeep commented May 10, 2019

This looks good. I had a branch that did the routing slightly differently. I didn't merge because I didn't want to cause more conflicts with this one, but I might revisit. I'm thinking this works just fine and maybe my potential refactor would be superfluous. I'll run it by you if I think it might be worthwhile to update. Shouldn't hold up this PR, though. Ship it!

@reggieriser reggieriser merged commit 9bf220d into master May 10, 2019
@reggieriser reggieriser deleted the mw-164911631-super-admin-user-can-query branch May 13, 2019 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants