Adding Authentication to API #12
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Added `BYPASS_SCHOOL_AUTH` as option to skip school auth (when developing) - Added auth service to deal with creating and checking validation - Added ensure user function to get or create a user based on schools auth info - Added /login to user controller - Updated user.schoolId to be unique in DB - Added new shared types (for use in client) - Enabled strict null checking (this should have been on from the start) - Fixed some errors that the compiler told me about once that was on - Potentially missing status codes in serializers - issues with querying in the users.service
Adds additional JWT fields for validation as well as ensures that the signing algorithms and keys are validated. Additional changes and cleanup to come. ---WIP--- Currently in a working state - however additional validation needs to be performed and lots of cleanup/restructure to follow. (Consider collapsing all the x.config.json files into a single yaml config file organized by domain with optional ENV var overrides for each?)
Fixed issue with json body not being seen on the request Updated developer provider settings Added developer login validations Updated login routes to follow above changes
Implements saml metadata endpoint for pulling SP metadata by the IDP
This was referenced Jun 13, 2021
- Added `BYPASS_SCHOOL_AUTH` as option to skip school auth (when developing) - Added auth service to deal with creating and checking validation - Added ensure user function to get or create a user based on schools auth info - Added /login to user controller - Updated user.schoolId to be unique in DB - Added new shared types (for use in client) - Enabled strict null checking (this should have been on from the start) - Fixed some errors that the compiler told me about once that was on - Potentially missing status codes in serializers - issues with querying in the users.service
daviddob
requested changes
Jun 16, 2021
# Conflicts: # .gitignore # .vscode/launch.json # @types/express/index.d.ts # config/default.yml.template # package-lock.json # package.json # scripts/generateConfig.sh # src/controller/login.controller.ts # src/controller/login.developer.controller.ts # src/controller/login.saml.controller.ts # src/environment.ts # src/index.ts # src/middleware/auth.middleware.ts # src/middleware/serializer/tests/users.serializer.test.ts # src/middleware/serializer/users.serializer.ts # src/migration/1623375449310-tightenUpUserModel.ts # src/router/login.developer.router.ts # src/router/login.router.ts # src/router/login.saml.router.ts # src/services/auth.service.ts # src/utils/passport.utils.ts # src/utils/passport/saml.passport.ts # tsconfig.json
Typeos from github Co-authored-by: David Dobmeier <[email protected]>
daviddob
approved these changes
Jun 23, 2021
daviddob
added a commit
that referenced
this pull request
Jun 23, 2021
Implemented initial developer auth + SAML flows for authentication, uses JWT for tokens Co-authored-by: Michael Drewitt <[email protected]>
daviddob
added a commit
that referenced
this pull request
Jun 23, 2021
Implemented initial developer auth + SAML flows for authentication, uses JWT for tokens Co-authored-by: Michael Drewitt <[email protected]>
daviddob
added a commit
that referenced
this pull request
Jun 23, 2021
Implemented initial developer auth + SAML flows for authentication, uses JWT for tokens Co-authored-by: Michael Drewitt <[email protected]>
jessehartloff
pushed a commit
that referenced
this pull request
Jun 29, 2021
Implemented initial developer auth + SAML flows for authentication, uses JWT for tokens Co-authored-by: Michael Drewitt <[email protected]>
jessehartloff
pushed a commit
that referenced
this pull request
Jun 30, 2021
Implemented initial developer auth + SAML flows for authentication, uses JWT for tokens Co-authored-by: Michael Drewitt <[email protected]>
jessehartloff
pushed a commit
that referenced
this pull request
Jul 19, 2021
Implemented initial developer auth + SAML flows for authentication, uses JWT for tokens Co-authored-by: Michael Drewitt <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
Adding auth providers to the api. Currently we've implemented a SAML provider (for school auth) as well as a local development only provider that allows you to input which user you want to login as. The configuration options for both of these things are controlled via
.ymlfiles in the newconfigdirectory.Due to using passport as a middleware for auth, we had to remove our user model from extending the express request type because it conflicted with passports (so stupid, we know). To compensate, we've decided that serializers will no longer act as middleware, and should instead just be called from the controller, and for controllers to (for the most part) become the last piece of middleware in the chain. After implementing this, tbh it probably should have been implemented this way from the start as it's much less obtuse as to what's going on in a controller.
In order to run this locally, you'll have to generate a config for yourself to encode and decode jwts. There's a new command, and documentation in the package json and readme respectively to learn yourself on how to make that happen.
I'm sure we're forgetting more than a few things in this list of stuff, so please please please ask lots and lots of questions.
There are sister PRs for both the client and shared repositories as well that will go up within in a few minutes of this PR going live, and I'd encourage everyone looking at this, to check that out as well.
Client PR - UBAutograding/devU-client#3
Shared PR -UBAutograding/devu-shared#1
Types of changes
Checklist
Further comments
This is a giant PR, and the absolute definition of "do as I say, not as I do". Under normal circumstances I wouldn't want to put up a PR that touches so much code. But because of how fundamental auth is to the underlying systems, it was fairly necessary.
What was less necessary was refactoring and changing the way that some of the project is structured and named, very sorry about that. Because the project is still in it's infancy, and this is our first "real" foray into the project, we noticed a bunch of things that should (and were) changed (mostly in regards to naming), to (hopefully) make the project easier to maintain, and easier to work in moving forward. It's just a shame these things weren't thought about until we really got into things.