[Hotfix][ALS-6861] BDC Auth showing as down on the BDC Graphana dashboard #186
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
Luke-Sikina
approved these changes
Jul 12, 2024
Luke-Sikina
pushed a commit
that referenced
this pull request
Oct 7, 2024
…oard (#186) * [ALS-6861] Optimize logging and enhance routing flexibility * [ALS-6861] Optimize logging and enhance routing flexibility * [ALS-6861] Remove logging
Gcolon021
added a commit
that referenced
this pull request
Oct 23, 2024
* [ALS-5508] Dependabot dep bumps - httpd - Jackson - deleted some commented out stuff * [ALS-5612] Create stored procedure to create a user (#154) * Add a new stored procedure to enable user creation * Specific database Auth * Update CreateUserWithRole stored procedure in auth DB The stored procedure, CreateUserWithRole, in the 'auth' database has been updated to improve user creation. It now checks for existing users and roles, and generates a new UUID if needed. Additionally, it associates new users with roles if they exist. * Rename CreateUserWithRole stored procedure file * Add general metadata parameter to CreateUserWithRole procedure * [ALS-5612] Updated stored procedure (#155) * Add connectionSubPrefix to user creation stored procedure * Refactor connection prefix manipulation in SQL procedure * Update user creation stored procedure The changes made address the process of user creation in the stored procedure. A new variable, @baseUUID, has been introduced for storing UUIDs during processing. Additionally, the preparation of @connectionSubPrefix has been adjusted to concatenate LONG_TERM_TOKEN with existing values instead of overriding them. * Add PIC-SURE User role assignment in CreateUserWithRole procedure Improved the CreateUserWithRole stored procedure in the auth-db. All new users are now automatically assigned the 'PIC-SURE User' role in addition to specific roles designated during account creation. This ensures all users have access to the base level of functionalities. * [ALS-0000] Fix wiring issues in auth app. * Java 21, Spring, code refactor and deploy as standalone container. (#163) **Deployment and Regression Testing**: The changes have been thoroughly regression tested and are currently live in the [Nhanes Production](https://nhanes.hms.harvard.edu/psamaui/login) environment. **Java Version**: - Updated to use Amazon Corretto Java 21.0.1. **Dependencies**: - Upgraded all dependencies to their latest versions, with Spring Boot updated to 3.2.4 and other Spring dependencies to 6.1.5. - Removed all unused dependencies. - Added new dependencies including Spring Data JPA, Spring Dev Tools, Spring Security, and Spring Web. **Docker Configuration**: - Introduced a development version for both Docker Compose and Dockerfile. - Transitioned to a multi-stage Dockerfile where the application is built using Maven, eliminating the need for a JRE on the build server. - Changed deployment from a WAR file on the PIC-SURE WildFly server to a standalone Docker container using Tomcat as the application server. **Application Configuration**: - Eliminated WildFly-specific configurations. - Replaced `standalone.xml` with `application.properties` for managing application variables and configurations. - Updated to use MySQL 8 connector. - Defined unprotected routes in the `SecurityConfig.java`. - Integrated application variables directly in classes using Spring's `@Value` annotation, removing the `JAXRSConfiguration` class. **Controller and Service Layers**: - Refactored `Services` that were functioning as controllers into true services, adding necessary `Controllers`. - Renamed all `Controllers` to include the keyword 'Controller' and ensured all services contain 'Service' in their names. - Controller Annotations: - `@API` -> `@Tag` - `@Path` -> `@RequestMapping` - `@ApiOperation` -> `@Operation` - `@Get`, `@Post`, `@Put`, `@Delete`, etc. -> `@GetMapping`, `@PostMapping`, `@PutMapping`, `@DeleteMapping`, etc. - `@RolesAllowed` -> `@Secured` - `@ApiParam` -> `@Parameter` - `@PathParam` -> `@PathParameter` - No annotation for request bodies -> `@RequestBody` required **Services**: - Separated business logic into a true service layer to improve organization and testability. - Utilized the `@Service` annotation for Spring auto-wiring. - Adopted `@Transactional` where necessary and removed the use of `BaseEntityService` in favor of Spring Data JPA. **Repositories**: - Transitioned all entity classes to use Spring Data JPA repositories, extending `JpaRepository`. **Entities and Models**: - Replaced `BaseEntity` with a local implementation to support Java 21. - Added models like `TokenInspection` and `CustomUserDetails` for enhanced authentication processes. **Enums and Utilities**: - Introduced a `SecurityRoles` enum to centralize role management. - Replaced outdated utility classes with `RestClientUtil` - updated `JWTUtil` JWT handling with Java 21 compatible libraries. **Filters and Unit Tests**: - Updated `JWTFilter` to support Java 21, Spring Web, and Spring Security. - Confirmed that all unit tests pass, with additional mocks implemented due to the transition to Spring Data JPA. - Increased unit test line coverage from **11%** to **61%** and method coverage from **14%** to **89%** for the service layer. I have increased the number of unit test cases from 29 to 176. * Merge Fence specific branch into release ### Build - Created `Jenkinsfile.fence`, which is used by Jenkins to build and upload this application to S3. ### AccessRule Merged both `AccessRule` entities. - Added two `AccessRule.TypeNaming` constants: `ALL_CONTAINS_OR_EMPTY` and `ALL_CONTAINS_OR_EMPTY_IGNORE_CASE`. - Migrated `subAccessRule` and `subAccessRuleParent` to a single `subAccessRule` property that uses a `@JoinTable` to represent the same relationship. - Added a new migration script to update existing `AccessRule` tables to use the new `JoinTable`. ### AccessRuleService Merged both `AccessRuleService` classes. - Updated all logic to account for the two new `AccessRule.TypeNaming` constants. - Refactored the AccessRule cache in the FENCE branch to use Spring's `@Cacheable` and `@CacheEvict`. ### AuthorizationService Merged both `AuthorizationService` classes. - Introduced a new property named `strictConnections`, which is assigned a comma-separated list of connection labels. These connections will be evaluated using a more strict authentication flow, requiring the user to have associated access rules and privileges. For non-strict connections, the user only needs associated privileges. - This approach may be revisited later. Standardizing on one approach is advisable. ### FenceAuthenticationService - Removed the existing `FenceAuthenticationService` and replaced it with the version from the Fence-specific branch. - Incorporated the most recent changes to Role, Privilege, AccessRule, and Fence Mapping into the `FenceAuthenticationService`, including the `FenceMappingUtility` class. ### Additional Authentication Services - Added `Okta0AuthAuthenticationService`, `OpenAuthenticationService`, and `StudyAccessService` to the merged branch. These services previously existed only in the Fence branch. ### Controllers - Refactored all service classes migrated from the Fence-specific branch to have associated `Controller` classes. ### Java 21 & Spring - Refactored all classes migrated from the Fence-specific branch to accommodate Java 21, Spring Boot 3, and Spring 6. ### Unit Test - Merged all unit tests from the Fence branch into related test classes or added them to the project. Ensured that no test changes were logically altered, providing confidence when merging and refactoring classes. The goal was to ensure no currently available test cases failed. * Remove unnecessary configurations from Jenkinsfile * Update logging level in Token and Authorization services The logging level in both the TokenService and AuthorizationService has been changed from info to debug. This change helps to manage the verbosity of logs, ensuring that only crucial information is logged at the info level. * Remove unnecessary logging * [ALS-6567] Add classes for fence_mapping.json Added three new model classes: BioDataCatalyst, StudyMetaData and IsHarmonizedDeserializer. These classes are used when parsing fence_mapping.json. * Update FenceMappingUtility with new model classes Updated the FenceMappingUtility.java and associated test cases to use new model classes: BioDataCatalyst and StudyMetaData. Additionally, roles are now created after the application Context is initialized. * Improve FENCEAuthenticationService with additional checks and code refactoring The code has been modified to include an additional check in FENCEAuthenticationService to ensure that FENCEMapping is not empty before creating access rules. This will prevent unnecessary errors and exception handling. * Update persistAll method to return list of roles The persistAll method in RoleService.java has been revised to return a list of Role objects after saving them. This change is also reflected in FENCEAuthenticationService.java where the returned list of new roles is now properly assigned to newRoles. * [ALS-6398] Enable secure and http-only flags for session cookies * [ALS-6103] Architectural changes to support multiple auth providers (#183) - Removed `OpenAuthenticationController`, `OktaAuthenticationController`, and `AuthController`. - Created the `AuthenticationService` interface. All authentication service classes **MUST** implement this interface. This interface contains three methods: `authenticate`, `getProvider()`, and `isEnabled()`. - `authenticate`: Implements the specific authentication logic for each authentication service. - `getProvider`: Returns the name of the provider, e.g., `fence`, `open`, `auth0`, etc. These values must correspond to the `idpProvider` value in the authentication controller path `/auth/authentication/{idpProvider}`. This value is used to look up the correct authentication service in the `AuthenticationServiceRegistry` class. - `isEnabled`: Returns `true` or `false` based on the corresponding `application.properties` value. - Created the `AuthenticationServiceRegistry`. This service registry maintains a map of all enabled `AuthenticationService` services and provides a `getAuthenticationService` method that returns an `AuthenticationService` based on the provider string. - Created the `AuthenticationController`. This controller uses the `AuthenticationServiceRegistry` to dynamically delegate authentication to the correct `AuthenticationService` based on the `{idpProvider}` path variable. * [ALS-6103] Update StudyAccessController with new annotations Added 'RolesAllowed' and 'RequestMapping' annotations to the StudyAccessController. The 'RolesAllowed' annotation replaces the 'Secured' one to specify authorized roles. The 'RequestMapping' annotation is added to specify the path for accessing study data. * [ALS-6103] Update StudyAccessController with new annotations (#184) Added 'RolesAllowed' and 'RequestMapping' annotations to the StudyAccessController. The 'RolesAllowed' annotation replaces the 'Secured' one to specify authorized roles. The 'RequestMapping' annotation is added to specify the path for accessing study data. * Resolve leading whitespace error programmatically (#185) * [Hotfix][ALS-6861] BDC Auth showing as down on the BDC Graphana dashboard (#186) * [ALS-6861] Optimize logging and enhance routing flexibility * [ALS-6861] Optimize logging and enhance routing flexibility * [ALS-6861] Remove logging * Update AuthorizationService (#187) * Update AuthorizationService Open access does not currently assign a connection to users. We should change this in the future, but for now we will handle the case. * Refactor open access role handling in UserService The revision refactors how open access roles are handled in UserService.java. A condition was added to check if the user's roles are null before assigning a new HashSet, thus improving code robustness. Instead of setting the roles directly, the role is now added to the user's existing roles, ensuring preservation of any pre-existing roles. * [ALS-6882] Fix migrator by dropping constraint * Remove AR drop * [CHORE] - Switch fetch type to EAGER * Update Dockerfile copies in maven settings.xml * Update Dockerfile move maven settings copy before mvn command... * Implement RAS Authentication Provider (#195) - Create `OktaAuthenticationService` by extracting generic OKTA authentication code. - Create & Implement RASAuthenticationService and RASAuthenticationServiceTest. - Create & Implement RASPassportService and RASPassportServiceTest. - Implement RAS Passport handling and polling. - Add new enum for potential passport validation responses. This is used to ensure all potential return values are handled with in a switch statement. - Add model for RAS Passport and RAS Visas. - Add `Passport` property to User entity. - Includes a corresponding migration script to add new column to database. - Add new methods to the JWTUtil class to assist with decoding and parsing RAS Passports which use a JWT format. - Included SessionService for session management with caching support. * Revert "Update Dockerfile" This reverts commit 67be2c6. * Revert "Update Dockerfile" This reverts commit 36ad515. * [ALS-7197] BDC Auth access: User able to access data (#203) * - The clinical query template has been updated to include the `parentAccessField` in the fields section. The `parentAccessionField` value is now set to `\\\\_Parent Study Accession with Subject ID\\\\`. - The `TOPMED+PARENT` access rule, associated with clinical privileges, has been fixed. The rule previously had misconfigured `subAccessRules` and `gates`: - The `TOPMED_CONSENT` gate consent rule was incorrectly set to `IS_EMPTY` instead of `IS_NOT_EMPTY`, causing queries to be erroneously rejected, even when the `"categoryFilters": "\\_topmed_consents\\"` section was correctly included in the query. - A missing `subAccessRule` named `ALLOW_TOPMED_CONSENT` was added. This rule ensures that all required consents are present in the `\\_topmed_consents\\` section of the query template. - A new method, `configureClinicalAccessRuleWithPhenoSubRule`, was added to `AccessRuleService`. This method contains the configuration for this access rule. - Previously, there were many unassociated access rules. All access rules are now correctly linked to other rules and privileges. In Spring Boot, any entity property defining a parent-child relationship must use the `cascade = CascadeType.ALL` option in the `@ManyToOne` or `@ManyToMany` annotations to ensure updates to the parent or child entities on save. - A new role, `MANUAL_ROLE_NAMED_DATASET`, has been assigned to all BDC users. This role grants users access to the `/named/dataset/`, `/named/dataset/{uuid}`, and `/query/{uuid}/metadata` endpoints. The role, along with its privilege and rule definition, is now part of the BDC infrastructure. - The `createPhoneTypeSubRule` method now correctly removes double-escaped characters. For example, a concept path formatted as `\\\\phs000xxx\\\\` is now converted to `\\phs000xxx\\`. - The `upsertConsentGate` method now properly escapes backslashes in rules. - The `extractAndCheckRule` method now correctly evaluates access rules that need to check map nodes. When using JsonPath to retrieve a node value, it is naturally converted into a list. This list is now converted into a Map for key checking. - When evaluating `subAccessRules` for a given rule, the rules must first be merged before evaluation. Since sub access rules may overlap with other rules, the method `preProcessARBySortedKeys` is used to handle the merging process. - The `pic-sure-auth-micro-app` was originally designed with an ephemeral database. With each deployment, all roles, privileges, and access rules were re-created. This provided an opportunity to add new allowed query types and standard access rules. Now that our database is persisted, there was no mechanism to add new allowed query types or standard access rules. A new method has been added to `PrivilegeService` named `updateAllPrivilegesOnStartup`. - This method updates all privileges to include all current standard access rules. Any standard access rules that need to be removed from a privilege must be handled via a migration script. - The method also updates all allowed query types. All existing allowed query types are removed from each `access_rule` that currently has allowed query types as `subAccessRules`. Once these are removed, all currently defined allowed query types are added to the `access_rule`, enabling the addition or removal of allowed query types without needing migration scripts. * Add transactional annotation to PrivilegeRepository * Change event listener and improve privilege update logic * Refactor privilege update method to avoid transactional query Replaced the custom query with direct calls to add access rules and save privileges. This simplifies the code and utilizes existing save method, ensuring transactional consistency. * Fix sub-access rule addition in PrivilegeService * Refactor role name constants to uppercase * Set logging level to DEBUG for auth service implementation Changed the logging level for `edu.harvard.hms.dbmi.avillach.auth.service.impl` from INFO to DEBUG to facilitate detailed debugging. This will help in tracking down issues more effectively during development and troubleshooting. * Switch logging level from DEBUG to TRACE in AccessRuleService Changed the logging level from DEBUG to TRACE for specific methods in AccessRuleService to reduce the verbosity of the application logs. This will make debugging more efficient by including detailed logs specific to fine-grain debugging levels without cluttering higher-level log outputs. * Restrict logging to denied access attempts Refactor log statement in AuthorizationService to log only when access is denied. This change reduces unnecessary log entries for granted access, improving log readability and performance. * Fix double backslash handling in Topmed access rule generation Address an issue where double backslashes in the concept path were not appropriately converted, causing potential mismatches in rule text generation. This ensures consistency by replacing all instances of `\\\\` with `\\` before forming the rule text. * Lower logging level in evaluateAccessRule method Changed log level from debug to trace for initial access rule evaluation steps. This change reduces verbosity in the logs, helping to focus on more critical debug information. * Update pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authorization/AuthorizationService.java * Enhance logging and configuration (#207) Updated logging statements to include user roles upon login in both RAS and FENCE authentication services. Parameterized logging levels in the application properties file for more flexible runtime configuration. * ALS-7306: Add expected result type auth for PFB (#208) * Fix token validation logic for long-term tokens (#209) * Fix token validation logic for long-term tokens * Fix token inspection logic and add logging for token refresh * Add long-term token check in isAuthorized method (#210) This commit adds a new parameter `isLongTermToken` to the `isAuthorized` method in the `AuthorizationService` class. It updates the `TokenService` and several test cases to accommodate this change, ensuring that long-term tokens bypass the session expiration check. * Update Jenkinsfile to use uppercase for LATEST_TAG (#212) Changed the value of LATEST_TAG from "latest" to "LATEST" to standardize the convention used for tagging. This ensures consistency across different environments and avoids potential issues with mismatched tag names. * [ALS-6921] Implement Open Access in PSAMA (#211) - The logout handler now returns a `200 OK` status upon success. - Added a new `OpenAccessController` that includes a single `open/validate` endpoint. - `JWTFilter` now permits the `open/validate` endpoint when using an application token. - Added a new `record` named `EvaluateAccessRuleResult`, which is returned from `passesAccessRuleEvaluation` after evaluating the access rules against the query. - Introduced a new method named `openAccessRequestIsValid`, which is used to validate open access requests. - Updated the callback URLs to use the new UI path `login/loading/`. - Removed `OpenAuthenticationService` and `OpenAuthenticationServiceTest` as they are no longer needed. - A new open access query template is returned for request that do not have an associated `uuid` or `user`. * Add deployment toggle to Jenkinsfile (#213) * Fix type for DEPLOY parameter in Jenkinsfile (#214) * Change Jenkins Parameter (#215) * Fix type for DEPLOY parameter in Jenkinsfile * Add logout endpoint and improve token validation (#217) Allow the "/logout" endpoint to be accessed without authentication to facilitate user sign-out. Additionally, enhance the logout handler by adding checks for token prefix and blank token to improve robustness. * Check for open access enabled (#218) * Add AR_DICTIONARY_REQUESTS (#219) * [ALS-6880] HttpClient with proxy * [ALS-6880] SLF4J dep issues * restTemplate connection refactoring * refactor to explicitly set proxy * Fix for error: 400 Bad Request: "invalid json" * [CHORE] int friendly default for RestClientConf * Update Dockerfile move maven settings copy before mvn command... * Add Javadoc comments to model classes (#223) Enhance code documentation by adding Javadoc comments to Ga4ghVisaV1, Ga4ghPassportV1, RasDbgapPermission, Passport, and User classes. These comments provide a clearer understanding of the purpose and structure of each class. * [CHORE] This is not an error * [ALS-7575] Add cache inspection and eviction services (#222) [ALS-7575] Added new endpoints for cache inspection and introduced CacheEvictionService to handle cache eviction for users. Security configuration was updated to allow access to cache endpoints, and several services were modified to utilize the new cache eviction mechanisms. * [ALS-7554] User should not be able to search without a valid token (#225) * Set token 'active' field to false on invalid token scenarios Ensure the 'active' field in tokenInspection is marked false whenever a token is invalid or user not found. This improves the clarity and consistency of token status checks throughout the service. * Remove custom key generator from cache eviction Removed the custom key generator from cache eviction in AccessRuleService for streamlined annotation usage. Added null and blank subject checks in CacheEvictionService to prevent errors, and removed unnecessary blank line in FENCEAuthenticationService. --------- Co-authored-by: Luke Sikina <[email protected]> Co-authored-by: Jeremy Nix <[email protected]> Co-authored-by: Luke Sikina <[email protected]> Co-authored-by: ramari16 <[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.
[Hotfix][ALS-6861] BDC Auth showing as down on the BDC Graphana dashboard