Conversation
7f9b31b to
f9be6fd
Compare
|
C'est ok pour les calls vers le MS signature. Reste à authentifier les tokens venant du MS |
809a7c6 to
424eec7
Compare
setup.py
Outdated
| "collective.eeafaceted.z3ctable", | ||
| "eea.facetednavigation", | ||
| "imio.helpers", | ||
| "imio.helpers>1.3.9", |
There was a problem hiding this comment.
TODO Faire une release 1.3.11 de imio.helpers
IMIO/imio.helpers#41
| token = auth_header.split(" ")[1] | ||
| except IndexError: | ||
| return False | ||
| return verify_auth_token(token, groups=["access_apims-esign"]) |
There was a problem hiding this comment.
@jeanmoulart Est-ce que c'est le bon groupe ? J'ai l'impression que ce n'est pas très logique. ça donnerait accès à tous les apps qui devraient contacter le MS signature, hors c'est plutôt le MS signature qui devra nous contacter
There was a problem hiding this comment.
Oui, ça devrait plutôt être access_ia-delib ou un truc du genre
There was a problem hiding this comment.
Ah oui, nickel, je viens de voir qu'ils ont eu la même problématique entre délib et vision. Ils ont mis à jour la doc en conséquence.
j'ai changé pour le groupe "access_imio-apps-docs". Il faudra adapter ça sur votre user keycloak apps. Vous pourrez tester les notifications avec JWT début de semaine prochaine je pense.
424eec7 to
d30cc29
Compare
d30cc29 to
66ca625
Compare
sgeulette
left a comment
There was a problem hiding this comment.
je ne fais pas le merge: en attente du merge de imio.helpers
66ca625 to
a3f518f
Compare
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughThis PR switches authentication from Basic auth (base64-encoded credentials) to Bearer token (JWT) authentication. It removes ESIGN_CREDENTIALS env var usage, updates create_external_session to obtain a Bearer token, enforces token verification in the microservice, and raises imio.helpers dependency to >1.3.10. Changes
Sequence DiagramsequenceDiagram
actor Client
participant BrowserView as Browser View
participant Utils as Session Utils
participant ExternalService as External Session Service
participant AuthLib as Auth Library\n(imio.helpers.ws)
Client->>BrowserView: request external session
BrowserView->>Utils: create_external_session(session_id, url)
Utils->>AuthLib: get_auth_token()
AuthLib-->>Utils: Bearer token
Utils->>ExternalService: POST /external_sessions\nAuthorization: Bearer {token}
ExternalService->>AuthLib: verify_auth_token(token)
alt token valid & in group
AuthLib-->>ExternalService: verified
ExternalService-->>Utils: session created (200)
else invalid or missing token
AuthLib-->>ExternalService: verification failed
ExternalService-->>Utils: 403 Forbidden
end
Utils-->>BrowserView: result
BrowserView-->>Client: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@setup.py`:
- Line 58: Update the dependency constraint in setup.py from
"imio.helpers>1.3.10" to "imio.helpers>=1.3.9" to allow the package that
introduced get_auth_token in 1.3.9; then locate any imports of verify_auth_token
(e.g., from imio.helpers or imio.helpers.ws) in the codebase and confirm the
correct module path (adjust imports to imio.helpers.ws if verify_auth_token
lives there or correct the name if it belongs elsewhere) so the import matches
the actual symbol location.
In `@src/imio/esign/services/external_session_feedback.py`:
- Around line 99-106: The code reads a private auth header and can pass an empty
token to verify_auth_token; replace use of self.request._auth with the public
header accessor (e.g., self.request.getHeader("Authorization")), ensure the
header starts with "Bearer ", split safely and reject empty tokens (i.e., after
splitting check token is truthy), and return False before calling
verify_auth_token if the token is missing or empty; update references to
auth_header and the token extraction logic around verify_auth_token accordingly.
In `@src/imio/esign/utils.py`:
- Around line 165-170: The requests.post call that assigns to ret (posting to
session_url with headers, data_payload and files_payload) is missing a timeout
and can hang; update the call in utils.py to pass a timeout parameter (e.g.,
timeout=10 or a configurable constant) to requests.post, or read a timeout from
existing configuration if available, so the function will raise on network
timeouts instead of blocking indefinitely.
🧹 Nitpick comments (2)
src/imio/esign/utils.py (1)
159-162: Consider handlingget_auth_token()failure.If
get_auth_token()fails or returns an invalid value, the request will proceed with a malformed Authorization header. Depending on howget_auth_token()behaves on failure (raises exception vs returns None), you may want explicit error handling here to provide a clearer error message.src/imio/esign/services/external_session_feedback.py (1)
109-115: Remove duplicate state documentation.This module-level docstring duplicates the information already present inside the class (lines 87-95). Consider keeping only one version to avoid maintenance burden.
Summary by CodeRabbit
New Features
Documentation
Dependencies