apply suggestions from code review

Author: jverswijver

CHANGELOG.md

@@ -3,92 +3,123 @@
 Observes [Semantic Versioning](https://semver.org/spec/v2.0.0.html) standard and [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) convention.
 
 ## [0.5.1] - 2022-09-27
+
 ### Added
+
 - Schema templating for insert queries using query params PR #131
+- Add support for OIDC login flow PR #130
 
 ## [0.5.0] - 2022-09-21
+
 ### Fixed
+
 - Bugs with returning UUID and NaN values PR #128
 
 ### Added
+
 - Support schemas with a `-` by specifying instead with `__` in dynamic spec PR #128
 - Support for new `antd-table` component. Prior `table` component will be deprecated in the next minor release. PR #128
 - Support for InsertComponent
 
 ## [0.4.1] - 2022-03-24
+
 ### Fixed
+
 - Bug with otumat version not being tied to the latest PR #119
 
 ## [0.4.0] - 2022-03-18
+
 ### Fixed
+
 - Bug with `order_by` not applying from fetch args PR #117
 
 ### Added
+
 - Support for new `slider` and `dropdown-query` components PR #118
 - Numpy parser for `component_interface.py` to remove numpy types for json serialization PR #118
 - Support for loginless mode PR #118
 
 ## [0.3.0] - 2022-01-21
+
 ### Changed
+
 - Hot-reload mechanism to use `otumat watch` PR #116
 - Renamed environment variable defining spec sheet to `PHARUS_SPEC_PATH` PR #116
 
 ### Added
-- Autoformatting strategy using `black` PR #116 
+
+- Autoformatting strategy using `black` PR #116
 - Support for sci-viz components `metadata`, `image`, `dynamic grid` PR #116
 - `component interface` for users to be able to load their own custom interface for sci-viz PR #116
 
 ### Fixed
+
 - Various bugs related to datetime PR #116
 
 ## [0.2.3] - 2021-11-18
+
 ### Added
+
 - Support for plot component PR #155
 - Fetch argument specification in `dj_query` PR #155
 
 ## [0.2.2] - 2021-11-10
+
 ### Fixed
+
 - Optimize dynamic api virtual modules. PR #113
 
 ## [0.2.1] - 2021-11-08
+
 ### Fixed
+
 - Error with retrieving the module's installation root path. PR #112
 
 ## [0.2.0] - 2021-11-02
+
 ### Added
+
 - Dynamic api generation from spec sheet.(#103, #104, #105, #107, #108, #110) PR #106, #109
 - `dynamic_api_gen.py` Python script that generates `dynamic_api.py`.
 - Add Tests for the new dynamic api.
 - `server.py` now loads the routes generated dynamically from `dynamic_api.py` when it is present.
 
 ## [0.1.0] - 2021-03-31
+
 ### Added
+
 - Local database instance pre-populated with sample data for `dev` Docker Compose environment. PR #99
 - Capability to insert multiple, update multiple, and delete multiple. PR #99
 - Allow dependency restriction to include secondary attributes from parent table. PR #99
 
 ### Changed
+
 - Update `datajoint` to newly released `0.13.0`. PR #97
 - Rename service `pharus` to `pharus-docs` in `docs` Docker Compose environment to allow simulataneous development. PR #99
 - Update NGINX reverse proxy image reference. PR #99
 - Refactored API design to align with common REST resource naming convention. (#38) PR #99
 - Hide classes and methods that are internal and subject to change. PR #99
 
 ### Removed
+
 - `InvalidDeleteRequest` exception is no longer available as it is now allowed to delete more than 1 record at a time. PR #99
 
 ### Fixed
+
 - `uuid` types not properly restricted on `GET /record`, `DELETE /record`, and `GET /dependency`. PR #102
 
 ## [0.1.0b2] - 2021-03-12
 
 ### Fixed
+
 - Fixed behavior where using list_table with a nonexistent schema_name creates it instead of returning an error message (#65) PR #63
 
 ### Changed
+
 - Contribution policy to follow directly the general DataJoint Contribution Guideline. (#91) PR #94, #95
 
 ### Added
+
 - Issue templates for bug reports and enhancement requests. PR #94, #95
 - Docker environment for documentation build. (#92) PR #94, #95
 - Add Sphinx-based documentation source and fix parsing issues. (#92) PR #94, #95
@@ -97,29 +128,36 @@ Observes [Semantic Versioning](https://semver.org/spec/v2.0.0.html) standard and
 ## [0.1.0b0] - 2021-02-26
 
 ### Security
+
 - Documentation with detail regarding warning on bearer token. (#83) PR #88
 
 ### Fixed
+
 - Incorrect virtual module reference of `schema_virtual_module` in table metadata. (#85) PR #88
 
 ### Added
+
 - Docker `dev` environment that supports hot reloading. PR #79
 - Documentation on setting up environments within `docker-compose` header. PR #79
 - `cascade` option for `/delete_tuple` route. (#86) PR #88
 - When delete with `cascade=False` fails due to foreign key relations, returns a HTTP error code of `409 Conflict` with a JSON body containing specifics of 1st child. (#86) PR #88
 
 ### Changed
+
 - Replaced `DJConnector.snake_to_camel_case` usage with `datajoint.utils.to_camel_case`. PR #88
 - Default behavior for `/delete_tuple` now deletes without cascading. (#86) PR #88
 - Consolidated `pytest` fixtures into `__init__.py` to facilitate reuse. PR #88
 - Modify dependency check to not perform deep check and use accessible fk relations only. (#89) PR #90
 - Update nginx image to pull from datajoint organization. (#80) PR #90
 
 ### Removed
+
 - Docker `base` environment to simplify dependencies. PR #79
 
 ## [0.1.0a5] - 2021-02-18
+
 ### Added
+
 - List schemas method.
 - List tables method.
 - Data entry, update, delete, and view operations for DataJoint table tiers: `dj.Manual`, `dj.Lookup`.
@@ -138,4 +176,4 @@ Observes [Semantic Versioning](https://semver.org/spec/v2.0.0.html) standard and
 [0.1.0]: https://github.com/datajoint/pharus/compare/0.1.0b2...0.1.0
 [0.1.0b2]: https://github.com/datajoint/pharus/compare/0.1.0b0...0.1.0b2
 [0.1.0b0]: https://github.com/datajoint/pharus/compare/0.1.0a5...0.1.0b0
-[0.1.0a5]: https://github.com/datajoint/pharus/releases/tag/0.1.0a5
+[0.1.0a5]: https://github.com/datajoint/pharus/releases/tag/0.1.0a5

pharus/interface.py

@@ -16,21 +16,21 @@ class _DJConnector:
     """Primary connector that communicates with a DataJoint database server."""
 
     @staticmethod
-    def _attempt_login(database_address: str, username: str, password: str):
+    def _attempt_login(databaseAddress: str, username: str, password: str):
         """
         Attempts to authenticate against database with given username and address.
 
-        :param database_address: Address of database
-        :type database_address: str
+        :param databaseAddress: Address of database
+        :type databaseAddress: str
         :param username: Username of user
         :type username: str
         :param password: Password of user
         :type password: str
         """
-        dj.config["database.host"] = database_address
+        dj.config["database.host"] = databaseAddress
         dj.config["database.user"] = username
         dj.config["database.password"] = password
-
+        print(dj.config, flush=True)
         # Attempt to connect return true if successful, false is failed
         dj.conn(reset=True)
 

pharus/server.py

@@ -5,6 +5,7 @@
 from typing import Callable
 from functools import wraps
 from typing import Union
+import traceback
 
 # Crypto libaries
 from cryptography.hazmat.primitives import serialization as crypto_serialization
@@ -62,16 +63,19 @@ def protected_route(function: Callable) -> Callable:
     def wrapper(**kwargs):
         try:
             if "database_host" in request.args:
-                jwt_payload = jwt.decode(
-                    request.headers.get("Authorization").split()[1],
-                    environ["PHARUS_OIDC_PUBLIC_KEY"],
-                    algorithms="RS256",
-                )
-                connect_creds = dict(
-                    databaseAddress=request.args["database_host"],
-                    username=jwt_payload[environ.get("PHARUS_OIDC_SUBJECT_KEY")],
-                    password=request.headers.get("Authorization").split()[1],
-                )
+                encoded_jwt = request.headers.get("Authorization").split()[1]
+                connect_creds = {
+                    "databaseAddress": request.args["database_host"],
+                    "username": jwt.decode(
+                        encoded_jwt,
+                        crypto_serialization.load_der_public_key(
+                            b64decode(environ.get("PHARUS_OIDC_PUBLIC_KEY").encode())
+                        ),
+                        algorithms="RS256",
+                        options=dict(verify_aud=False),
+                    )[environ.get("PHARUS_OIDC_SUBJECT_KEY")],
+                    "password": encoded_jwt,
+                }
             else:
                 connect_creds = jwt.decode(
                     request.headers.get("Authorization").split()[1],
@@ -80,7 +84,7 @@ def wrapper(**kwargs):
                 )
             return function(connect_creds, **kwargs)
         except Exception as e:
-            return str(e), 401
+            return traceback.format_exc(), 401
 
     wrapper.__name__ = function.__name__
     return wrapper
@@ -195,13 +199,6 @@ def login() -> dict:
         :statuscode 500: Unexpected error encountered. Returns the error message as a string.
     """
     if request.method == "POST":
-        # Check if request has the correct fields
-        # if ( "database_host" not in request.headers and
-        #     not request.json.keys() >= {"databaseAddress", "username", "password"}
-
-        # ):
-        #     return dict(error="Invalid Request, check headers and/or json body")
-
         # Try to login in with the database connection info, if true then create jwt key
         try:
             if "database_host" in request.args:
@@ -227,26 +224,31 @@ def login() -> dict:
                     headers=headers,
                     auth=auth,
                 )
-                access_token = result.json()["access_token"]
-                _DJConnector._attempt_login(
-                    request.args["database_host"],
-                    jwt.decode(access_token, environ.get("PHARUS_OIDC_PUBLIC_KEY")),
-                    access_token,
-                )
-                return dict(jwt=access_token)
+                encoded_jwt = result.json()["access_token"]
+                connect_creds = {
+                    "databaseAddress": request.args["database_host"],
+                    "username": jwt.decode(
+                        encoded_jwt,
+                        crypto_serialization.load_der_public_key(
+                            b64decode(environ.get("PHARUS_OIDC_PUBLIC_KEY").encode())
+                        ),
+                        algorithms="RS256",
+                        options=dict(verify_aud=False),
+                    )[environ.get("PHARUS_OIDC_SUBJECT_KEY")],
+                    "password": encoded_jwt,
+                }
             else:  # Database login
-                _DJConnector._attempt_login(
-                    request.json["databaseAddress"],
-                    request.json["username"],
-                    request.json["password"],
-                )
                 # Generate JWT key and send it back
                 encoded_jwt = jwt.encode(
                     request.json, environ["PHARUS_PRIVATE_KEY"], algorithm="RS256"
                 )
-                return dict(jwt=encoded_jwt)
+                connect_creds = request.json
+            if connect_creds.keys() < {"databaseAddress", "username", "password"}:
+                return dict(error="Invalid Request, check headers and/or json body")
+            _DJConnector._attempt_login(**connect_creds)
+            return dict(jwt=encoded_jwt)
         except Exception as e:
-            return str(e), 500
+            return traceback.format_exc(), 500
 
 
 @app.route(f"{environ.get('PHARUS_PREFIX', '')}/schema", methods=["GET"])