WIP:Cas client #66
WIP:Cas client #66
Conversation
| */ | ||
| public class CasClientProperties { | ||
|
|
||
| private String proxyTicketQueryKey = "ticket"; |
There was a problem hiding this comment.
You must use getArtifactParameter() from ServiceProperties https://github.com/spring-projects/spring-security/blob/master/cas/src/main/java/org/springframework/security/cas/ServiceProperties.java#L99
Each it should be an existing bean https://github.com/kakawait/cas-security-spring-boot-starter/blob/master/cas-security-spring-boot-autoconfigure/src/main/java/com/kakawait/spring/boot/security/cas/CasSecurityAutoConfiguration.java#L65
There was a problem hiding this comment.
Thus I think this class could be removed
| * @author Jonathan Coueraud | ||
| */ | ||
| @Qualifier("casClientHttpRequestInterceptor") | ||
| public class CasStatefulInterceptor implements ClientHttpRequestInterceptor { |
There was a problem hiding this comment.
I will be moreStatefulCasInterceptor rather than CasStatefulInterceptor to keep logic from most specific to less specific, I mean Stateful vs Stateless then is about Cas and then is about Interceptor
| /** | ||
| * @author Jonathan Coueraud | ||
| */ | ||
| public interface AuthenticatedPrincipal { |
There was a problem hiding this comment.
I don't really fan of that name because it look like that interface hold the AuthenticatedPrincipal where is just an interface that abstract how to retrieve the Principal.
For now I don't have other name but I would think about but the idea will be more around PrincipalExtractor (but I don't like that name too)
| import java.lang.annotation.*; | ||
|
|
||
| /** | ||
| * @author Jonathan Coueraud |
There was a problem hiding this comment.
I understand that you want to become famous but I think only one @author comment is enough 🤣
| @Bean | ||
| @ConditionalOnMissingBean | ||
| public AuthenticatedPrincipal authenticatedPrincipal() { | ||
| return () -> (Principal) SecurityContextHolder.getContext().getAuthentication().getPrincipal(); |
There was a problem hiding this comment.
Can throw NPE, maybe do better check of null and RuntimeException
| /** | ||
| * @author Jonathan Coueraud | ||
| */ | ||
| public interface CookieWrapper { |
There was a problem hiding this comment.
Why do you need CookieWrapper than using existing HttpCookie class?
|
|
||
| private static final CurrentTimeMillisAdapter CURRENT_TIME_MILLIS_ADAPTER = new CurrentTimeMillisAdapter(){}; | ||
|
|
||
| private final ConcurrentHashMap<HttpContextId, Map<String, CookieWrapper>> contextIdToCookies = |
There was a problem hiding this comment.
Can't be mapped to Map<HttpContextId, Map<String, CookieWrapper>>?
| private final ConcurrentHashMap<HttpContextId, Map<String, CookieWrapper>> contextIdToCookies = | ||
| new ConcurrentHashMap<>(); | ||
|
|
||
| private final ConcurrentHashMap<Principal, Set<URI>> principalToUris = |
There was a problem hiding this comment.
Can't be mapped to Map<Principal, Set<URI>>?
|
|
||
| private final DelayQueue<CookieExpiry> expiryQueue = new DelayQueue<>(); | ||
|
|
||
| private final ConcurrentHashMap<String, CookieExpiry> expiryMap = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
Can't be mapped to Map<String, CookieExpiry>?
| /** | ||
| * @author Jonathan Coueraud | ||
| */ | ||
| public class ProxyTicket implements Ticket { |
There was a problem hiding this comment.
Please use Lombok for getter, equals & hashcode
No description provided.