implement /internal/session endpoint #595

ethaaalpha · 2025-09-24T15:09:44Z

Firstly, this pull request is based on another one: multi_api.
This one introduce the new /internal/session endpoint, which implement the DELETE, POST, GET methods.

Spring Security

A new SecurityFilterChain is present:InternalSecurityConfig, it handles all /internal/** urls:

CORS policy is enabled
CSRF Token verification is enabled:
The token will be defined with set_cookie after a successful login.
All of the "non secure" http requests will need the CSRF Token except for POST on /internal/session.
vip-session-cookie and vip-user-cookie are defined on login and use by the filter chain to check authentication

Cookies and actual frontend endpoints evolutions

By the way, we changed the cookies to httpOnly and adapt the actual frontend to support them in this case. It mean that the actual frontend will use GET /internal/session to determine if the user is actually logged (the current sign-in also use the new endpoint).
Some small modifications were made in GWT serverside code to support httpOnly.
The cookies are also set as secure if the apache.sslport properties is not 80.

Others

Some new annotations were introduced for the deprecation of SmartGWT (@VIPRemoval and @VIPCheckRemoval)

- add new `InternalSecurityConfig` filter chain (defined as order: 2) - created DELETE/POST/GET methods for /internal/session - move some models from vip-api to vip-core - create specific class for `CurrentUserProvider` service

- add a new `SessionAuthenticationProvider` with associated Filter/Token - refractor `ApikeyAuthenticationProvider` and `SessionAuthenticationProvider` into `AbstractAuthenticationProvider` - POST /internal/session now return Session object as valid response - rename `SpringApiUser` to `SpringPrincipalUser`

- make COOKIES_SESSION and COOKIES_USER `httpOnly` -- adapt GWT code to support httpOnly on server implementation - cookies will set as `secure` only if *apache.sslport* != 80 - edit sign-in.js/home.js endpoints from */rest/session* to */internal/session* for sign-in and retrieving current session instead of cookies (due to **httpOnly**)

- VIPRemoval and VIPCheckRemoval created annotations

- all unsafe methods are concerned under /internal/** except for /internal/session (which creates the token)

- refactor `SessionControler` to handle http/cookie related objects instead of `SessionBusiness` - make signIn define SecurityContextHolder

ngeorges-cnrs

Works for me in general, I just made minor comments on some code.

Two points to be discussed with @axlbonnet :

the /internal API response body on errors is not final in the present PR, this is normally not a blocker, as it can be handled later.
server.getApacheSSLPort() != 80 as the condition for httpOnly cookies should probably be replaced by a dedicated new setting (and getApacheSSLPort removed as unused ?). This is used in two places where cookies are set, VipSessionBusiness.java and SessionController.java.

ngeorges-cnrs · 2025-09-26T13:56:51Z

vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/controller/SessionController.java

+        this.server = server;
+    }
+
+    @GetMapping()


Minor syntax inconsistency : we typically do not write parenthesis for annotations with no parameters. This also applies to @PostMapping() below.

ngeorges-cnrs · 2025-09-26T14:01:57Z

vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/controller/SessionController.java

+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RestController;
+
+import fr.insalyon.creatis.vip.core.client.view.CoreConstants;


New controllers for the internal API should ideally have no dependency on GWT-related packages, so any import of *.client.* or *.server.rpc.* should in theory induce a refactoring. This could also be handled a as separate task later.

Yes, we'll handle that latter. I specific constants and models refactoring is needed.

ngeorges-cnrs · 2025-09-26T14:09:59Z

vip-portal/src/main/webapp/js/home.js

+    fetch("/internal/session").then(function (response) {
+        if (response.status != 200) {
+            window.location = "index.html";
+        }


Minor: maybe use response.ok instead of response.status ?

Also, some notes on JS fetch() error handling, which will be more important for the JS front-end :

a 4XX/5XX response other than 401 technically doesn't indicate that the user is disconnected, so showing the login page in that case is not great, especially on 5XX. Some user-visible error message while staying on "home" might be preferable.

there is a similar problem in case of network errors, where the promise won't resolve and which can be handled with .catch()

Okay, I've changed to response.ok.
About the JS notes, I'll write then down into a ticket of @hippolyteblot to make sure these issues will be handled by the new front.

axlbonnet

Seems good except for a few remarks.

Also, it would be nice to check if spring security login/session/cookie could replace some stuff here.

axlbonnet · 2025-10-03T08:58:52Z

vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/business/VipSessionBusiness.java

            User user) throws BusinessException, CoreException {
        try {
+            // see explanation in SessionBusiness -> createCookie
+            boolean isSecure = server.getApacheSSLPort() != 80;


This config is (almost) unused and we'll get rid of it.
Maybe introduce a new "secure" or "development" config that could be used be several things.

I created the "dev" property which is by default false.

axlbonnet · 2025-10-03T09:00:05Z

vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/business/VipSessionBusiness.java


 @Service
 @Transactional
+@VIPRemoval


As several methods are @VIPCheckRemoval I think the whole class should be too.

axlbonnet · 2025-10-03T09:28:38Z

...java/fr/insalyon/creatis/vip/core/server/security/common/AbstractAuthenticationProvider.java

+
+// this class was factorized from the Apikey Security provider 
+// since we have added the Session Security Provider
+@Service


i don't think @service is necessary here as it is an abstract class

axlbonnet · 2025-11-10T12:30:01Z

vip-portal/src/main/java/fr/insalyon/creatis/vip/portal/client/Main.java

        String session = null;

+        // this will actually always return null since we use httpOnly cookies
+        // the actual values are retrieved in the service implementation itself


you can remove it then

axlbonnet · 2025-11-10T12:36:41Z

vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/rpc/ConfigurationServiceImpl.java

            logger.debug("VIP successfully configured.");

-            if (configurationBusiness.validateSession(email, session)) {
+            User user = vipSessionBusiness.resetSessionFromCookie(getThreadLocalRequest());


remove email and session in method signature

axlbonnet · 2025-11-10T12:38:59Z

vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/controller/SessionController.java

+            response.addCookie(createCookie(CoreConstants.COOKIES_USER, null, 0, true));
+            response.addCookie(createCookie(CoreConstants.COOKIES_CSRF_TOKEN, null, 0, false));
+        } catch (BusinessException e) {
+            throw new ApiException(e); // change


change indeed

Done in #602.

axlbonnet · 2025-11-10T12:40:23Z

vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/controller/SessionController.java

+        Cookie cookie = new Cookie(name, value);
+        // for the moment we use that, but a property might be created in vip.conf
+        // instead!
+        boolean isSecure = server.getApacheSSLPort() != 80;


in development, we may need not httpOnly and not Secure ?

In dev we only want to change secure (HTTPS) to false (HTTP). But the behavior of httpOnly is still wanted and needed.

axlbonnet · 2025-11-10T12:41:43Z

vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/business/SessionBusiness.java

+        // define authenticated user in Spring context for the current request
+        // this is usefull to call getSession()
+        SecurityContextHolder.getContext()
+                .setAuthentication(sessionAuthenticationProvider.createAuthenticationFromUser(user));


Just remove spring security stuff here and create a new session to make it simpler..

axlbonnet · 2025-11-17T07:58:25Z

vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/business/VipSessionBusiness.java

            sessionCookie.setPath("/");
+            sessionCookie.setHttpOnly(true);
+            sessionCookie.setSecure(isSecure);
+


Could the cookie stuff be factorized with the stuff from Session controller ?

Since this class will disappear after the GWT migration, I don't think this is really useful?

axlbonnet · 2025-11-17T08:04:00Z

vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/controller/SessionController.java

+            response.addCookie(createCookie(
+                    CoreConstants.COOKIES_CSRF_TOKEN, token.getToken(), CoreConstants.COOKIES_DURATION, false));
+
+            return session;


I think the cookies are set at login but never renewed.
So I guess that after 1 week, the user will have to login again ?

axlbonnet · 2025-11-17T08:26:49Z

vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/controller/SessionController.java

-        session.password = null;
-        session.level = UserLevel.Beginner;
-        return session;
+        return sessionBusiness.getSession();


Maybe renew the cookies here to avoid them expiring after 1 week.

ethaaalpha added 9 commits September 15, 2025 13:55

basic session authentification for internal api

9d4f213

- add new `InternalSecurityConfig` filter chain (defined as order: 2) - created DELETE/POST/GET methods for /internal/session - move some models from vip-api to vip-core - create specific class for `CurrentUserProvider` service

introduction of migration code annotations

98622ce

- VIPRemoval and VIPCheckRemoval created annotations

add csrf token support

d32f992

- all unsafe methods are concerned under /internal/** except for /internal/session (which creates the token)

delete csrf token on logout

58aa524

code improvements

ce82c57

- refactor `SessionControler` to handle http/cookie related objects instead of `SessionBusiness` - make signIn define SecurityContextHolder

SessionController integration tests

0aee007

enable cors

2a9e298

ethaaalpha requested a review from ngeorges-cnrs September 24, 2025 15:11

ngeorges-cnrs reviewed Sep 26, 2025

View reviewed changes

pr review

0ab16dd

ethaaalpha marked this pull request as ready for review September 29, 2025 08:41

Merge branch 'develop' into internal_session

36b2d5a

ethaaalpha mentioned this pull request Oct 7, 2025

implement /internal/applications endpoint #600

Draft

axlbonnet reviewed Nov 17, 2025

View reviewed changes

implement /internal/session endpoint #595

Are you sure you want to change the base?

implement /internal/session endpoint #595

Uh oh!

Conversation

ethaaalpha commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Spring Security

Cookies and actual frontend endpoints evolutions

Others

Uh oh!

ngeorges-cnrs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ethaaalpha Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

axlbonnet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ethaaalpha Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ethaaalpha commented Sep 24, 2025 •

edited

Loading

ethaaalpha Sep 29, 2025 •

edited

Loading

ethaaalpha Nov 17, 2025 •

edited

Loading