[GitHub] [syncope] mmoayyed commented on pull request #362: Upgrading Spring Boot to 2.7 and CAS to 6.6

[GitBox <gi...@apache.org>]

mmoayyed commented on PR #362:
URL: https://github.com/apache/syncope/pull/362#issuecomment-1193363891

   I don't think downgrading is a good or practical option. First, it's possible that we might break someone else, and also something in CAS UI that might depend on this, but the bigger concern is that downgrading is only a stop-gap that sooner or later would break. The strategy should be that if a CAS module decides to switch a dependency, and for whatever reason SB does not, then we ought to be able to receive that update immediately and easily without a lot of gymnastics. This quickly becomes important when security patches to external libs that might affect CAS but not SB would be needed.
   
   Short of declaring a dependency on the dialect, (or even declaring an override version), both of which I think are uncomfortable, I would think this should be the more ideal solution would be putting this into wa or wa/starter:
   
   ```xml
   <dependencyManagement>
       <dependencies>
         <dependency>
           <groupId>org.apereo.cas</groupId>
           <artifactId>cas-server-support-bom</artifactId>
           <version>${cas.version}</version>
           <type>pom</type>
           <scope>import</scope>
         </dependency>
       </dependencies>
     </dependencyManagement>
   ```
   
   We should let CAS manage CAS, and then we only override what we want, and only when we want it. Not because we have to. 
   
   As for the 401 vs 500 error, I don't think you're missing anything. The scenario is that generally, the external provider returns back a 401, and CAS translates that, *correctly* I think into a 401. One could debate whether the end result should be a 401 or 500, and In my opinion, 500 makes more sense, though I don't recall if this was ever an intentional change. That said, as far as the particular test here goes, it should be safe to assume that the response status returned from the test is either a 4xx or a 5xx. Both should be appropriate. You should care about the fact that the operation failed in the general sense. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@syncope.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org