You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This document describes the solution implementations for the following issues in Konsulin API.
Issue 1: Enrich SuperTokens Profile Metadata with Roles and FHIR Resource IDs
Problem
SuperTokens profiles did not provide sufficient metadata for direct authorization checks. Whenever resource ownership validation was needed, the system had to perform additional queries to the database or FHIR server, causing overhead and latency.
const (
supertokenAccessTokenPayloadRolesKey="st-role"supertokenAccessTokenPayloadRolesValueKey="v"supertokenAccessTokenPayloadFhirResourceId="fhirResourceId"// New
)
New helper: getFhirResourceIdForUser(ctx, userID, roles) (string, error)
It derives the FHIR resource ID from the user’s roles with this priority:
Practitioner → Practitioner/{ID} (highest)
Patient → Patient/{ID}
Otherwise → Person/{ID}
Session creation change: The CreateNewSession override adds fhirResourceId to the access token payload by calling getFhirResourceIdForUser() and setting accessTokenPayload[supertokenAccessTokenPayloadFhirResourceId].
New constants: keyFHIRResourceId, supertokenAccessTokenPayloadFhirResourceId
SessionOptional middleware: Reads fhirResourceId from the access token payload and stores it in the request context (keyFHIRResourceId, CONTEXT_FHIR_RESOURCE_ID).
Unit: getFhirResourceIdForUser() for different role combinations and priority (Practitioner > Patient > Person).
Integration: Access token payload after login; context values in middleware.
E2E: API calls with the new token metadata and authorization checks.
Issue 2: Use FHIR Bundle for Batch PractitionerRole Availability Updates
Problem
The PractitionerAvailabilityEditor updated multiple PractitionerRole resources one-by-one in a loop. If one update failed mid-batch, earlier updates were already persisted, leading to an inconsistent state that was hard to recover.
Solution
The availability update flow was refactored to use a FHIR Bundle transaction: the client sends one request with all PractitionerRole updates in a Bundle, and the backend processes it atomically (all-or-nothing). The backend already had the infrastructure to support FHIR Bundle transactions.
Auth uses scanBundle() to validate every entry in the Bundle (resourceType, RBAC, resource ownership, structure). If any entry fails, the whole request is rejected.
Request: Frontend → POST /fhir (Bundle) → Auth → Bridge → FHIR server
Validation: Auth detects Bundle, runs scanBundle() on each entry (RBAC + ownership). If all pass → Bridge forwards; otherwise → error.
Transaction: FHIR server processes the Bundle atomically (all succeed or all roll back); response is returned to the client.
Benefits
Atomicity: All availability updates succeed or fail together; no partial updates.
Consistency: No inconsistent state on partial failure; automatic rollback.
Performance: One network round-trip instead of many sequential requests.
Reliability: Clear success/failure and simpler error handling.
Flow Comparison
Before (sequential)
Update #1 ✅ → #2 ✅ → #3 ❌ → inconsistent state; user must fix manually.
After (Bundle)
Single Bundle → all succeed or all fail → consistent state; user can retry the whole operation.
Error Handling
If any entry fails validation or the FHIR server rejects the transaction, the backend can return an OperationOutcome; the FHIR server rolls back the whole transaction. The frontend gets a single error and can retry the full batch.
Sessions and access tokens now include a user FHIR resource ID when available; token issuance performs a read-only lookup to attach the best-matching resource (Practitioner, Patient, or Person).
Improved magic-link/email and SMS delivery flows with validation and timeout handling.
Refactor
Unified session initialization and consistent population of typed context keys for roles, UID, and FHIR resource ID across request handling.
Updated handlers and services to read identity and roles from the unified context keys.
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
Use the following commands to manage reviews:
@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.
Use the checkboxes below for quick actions:
▶️ Resume reviews
🔍 Trigger review
📝 Walkthrough
Walkthrough
Adds a typed context key type and a new FHIR resource ID context key; SuperTokens hooks now resolve and embed a user's FHIR resource ID into the access-token payload; middleware extracts that FHIR resource ID (and typed role/uid keys) into request context; user usecase exposes a read-only FHIR ID lookup.
Changes
FHIR resource ID propagation & typed context keys
Layer / File(s)
Summary
Data Shape / Constants internal/pkg/constvars/internal_app.go, internal/app/delivery/http/middlewares/session.go
Adds exported type ContextKey string and CONTEXT_FHIR_RESOURCE_ID ContextKey = "fhir_resource_id"; introduces typed local context-key constants and supertokenAccessTokenPayloadFhirResourceId = "fhirResourceId".
Public API (contracts) internal/app/contracts/user.go
Adds LookupUserFHIRResourceIDsInput and extends UserUsecase with LookupUserFHIRResourceIDs(...) (read-only lookup for existing FHIR IDs).
Implements LookupUserFHIRResourceIDs: validates input, queries Practitioner/Patient/Person by SuperTokenUserID, returns any found IDs in InitializeNewUserFHIRResourcesOutput, logs lookup errors without creating resources.
Auth service / SuperTokens hooks internal/app/services/core/auth/auth_supertoken_impl.go
Adds getFhirResourceIdForUser(ctx,userID,roles) with timeout and role-priority selection; refactors SuperTokens overrides into helper wrappers: supertokenCreateCode, supertokenConsumeCode, supertokenCreateNewSession, delivery overrides, contact validators, and ensureRoleExists; attaches fhirResourceId into access-token payload (empty string on lookup error).
SessionOptional now initializes defaults for roles, uid, and fhirResourceId; when session exists, reads roles and fhirResourceId from access token payload; sets typed context keys (keyRoles, keyUID, keyFHIRResourceId) and exported constvars keys into request context for downstream handlers.
Replaces in-context string key usages with typed constvars.CONTEXT_FHIR_ROLE and constvars.CONTEXT_UID when reading roles/uid; webhook actor ID derivation now reads UID via CONTEXT_UID.
Updates TestAuthRouter_ContextPropagation expectations to assert context values via middlewares.ContextAPIKeyAuth, constvars.CONTEXT_FHIR_ROLE, and constvars.CONTEXT_UID instead of raw string keys.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name
Status
Explanation
Resolution
Docstring Coverage
⚠️ Warning
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.
Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name
Status
Explanation
Description Check
✅ Passed
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
✅ Passed
The title clearly summarizes the main change: adding FHIR resource ID handling in session middleware and authentication components.
Linked Issues check
✅ Passed
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
✅ Passed
Check skipped because no linked issues were found for this pull request.
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches🧪 Generate unit tests (beta)
Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Here are some key observations to aid the review process:
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 Security concerns
Token payload trust: Middleware trusts access token claims (roles and fhirResourceId) as-is. Ensure SuperTokens verification guarantees integrity and the keys used are namespaced to avoid collisions. Also, be cautious logging fhirResourceId and user_ids; while not secrets, they are identifiers—confirm logs comply with privacy policies.
getFhirResourceIdForUser initializes or fetches FHIR resources on every session creation and uses a 10s deadline. This can add latency to login/refresh and put load on the FHIR/DB backends. Consider caching the computed resource ID or moving this to a lazy fetch outside the hot path, and avoid creating resources unnecessarily.
// getFhirResourceIdForUser determines the FHIR resource ID based on user's roles and FHIR resource IDs// Priority: Practitioner > Patient > Personfunc (uc*authUsecase) getFhirResourceIdForUser(ctx context.Context, userIDstring, roles []string) (string, error) {
// Initialize FHIR resources inputinitFHIRResourcesInput:=&contracts.InitializeNewUserFHIRResourcesInput{
SuperTokenUserID: userID,
}
initFHIRResourcesInput.ToogleByRoles(roles)
// Get or create FHIR resourcesinitializeResourceCtx, initializeResourceCtxCancel:=context.WithDeadline(ctx, time.Now().Add(10*time.Second))
deferinitializeResourceCtxCancel()
initializedResources, err:=uc.UserUsecase.InitializeNewUserFHIRResources(initializeResourceCtx, initFHIRResourcesInput)
iferr!=nil {
uc.Log.Error("authUsecase.getFhirResourceIdForUser error initializing FHIR resources",
zap.Error(err),
)
return"", err
}
// Determine FHIR resource ID based on role priority:// 1. If Practitioner role exists → Practitioner/{ID}// 2. Else if Patient role exists → Patient/{ID}// 3. Otherwise → Person/{ID}for_, role:=rangeroles {
ifrole==constvars.KonsulinRolePractitioner&&initializedResources.PractitionerID!="" {
returnfmt.Sprintf("Practitioner/%s", initializedResources.PractitionerID), nil
}
}
for_, role:=rangeroles {
ifrole==constvars.KonsulinRolePatient&&initializedResources.PatientID!="" {
returnfmt.Sprintf("Patient/%s", initializedResources.PatientID), nil
}
}
ifinitializedResources.PersonID!="" {
returnfmt.Sprintf("Person/%s", initializedResources.PersonID), nil
}
return"", errors.New("no FHIR resource ID found for user")
}
CreateNewSession calls getFhirResourceIdForUser with context.Background(), losing request cancellation/timeouts and trace info. Pass through the provided userContext or a request-scoped context to ensure proper cancelation and observability.
// Get FHIR resource ID for the user based on their rolesctx:=context.Background()
fhirResourceId, fhirErr:=uc.getFhirResourceIdForUser(ctx, userID, userRoles)
iffhirErr!=nil {
uc.Log.Error("authUsecase.CreateNewSession error getting FHIR resource ID",
zap.String("user_id", userID),
zap.Error(fhirErr),
)
accessTokenPayload[supertokenAccessTokenPayloadFhirResourceId] =""
} else {
accessTokenPayload[supertokenAccessTokenPayloadFhirResourceId] =fhirResourceIduc.Log.Info("authUsecase.CreateNewSession added FHIR resource ID to access token",
zap.String("user_id", userID),
zap.String("fhir_resource_id", fhirResourceId),
)
}
Two context keys carry roles: untyped keyRoles and typed CONTEXT_FHIR_ROLE. This duplication can cause confusion and drift. Prefer a single typed key and plan removal of the deprecated untyped keys promptly.
ctx:=context.WithValue(r.Context(), keyRoles, roles)
ctx=context.WithValue(ctx, keyUID, uid)
ctx=context.WithValue(ctx, keyFHIRResourceId, fhirResourceId)
// new keys for context will be used for now and one and this// will deprecate the use of untyped string in context keysctx=context.WithValue(ctx, constvars.CONTEXT_FHIR_ROLE, roles)
ctx=context.WithValue(ctx, constvars.CONTEXT_UID, uid)
ctx=context.WithValue(ctx, constvars.CONTEXT_FHIR_RESOURCE_ID, fhirResourceId)
Inconsistent context key population in EnsureAnonymousSession.
This middleware sets keyRoles and keyUID but doesn't set the new keyFHIRResourceId or the typed constvars.CONTEXT_* keys that SessionOptional now provides. Downstream handlers expecting CONTEXT_FHIR_RESOURCE_ID may encounter missing context values.
The reason will be displayed to describe this comment to others. Learn more.
Mau tanya mas @gilanglahat22 , Apakah ada alasan khusus kenapa memilih reuse function InitializeNewUserFHIRResources ketimbang membuat function baru yang khusus untuk melakukan query FHIR resources yang dimiliki oleh requester?
Kalau dilihat secara internal dari InitializeNewUserFHIRResources, focus function tersebut kan write operation, sedangkan yang dibutuhkan di bagian ini seharusnya read operations. Kemudian juga secara penamaan, function InitializeNewUserFHIRResources sudah jelas tujuannya tidak diperuntukan untuk mengambil FHIR resources berdasarkan SuperTokenUserID
The reason will be displayed to describe this comment to others. Learn more.
Thanks masukannya Mas @luckyAkbar . Setuju, kalau memakai InitializeNewUserFHIRResources di flow ini memang kurang tepat karena function tersebut bisa melakukan write operation.
Sudah saya cek di current implementation, flow ini sekarang memakai LookupUserFHIRResourceIDs, yang sifatnya read-only dan hanya query resource Practitioner/Patient/Person yang sudah ada berdasarkan SuperTokenUserID. Function initialize/create tetap dipakai hanya di flow yang memang bertujuan untuk inisialisasi resource user.
The reason will be displayed to describe this comment to others. Learn more.
this comment is not a change request to the code, just for discussion
mas @lamurian , aku recheck current issue yang akan di solve oleh PR ini dan secara teknis ini adalah function utama yang akan mengambil FHIR resources yang dimiliki oleh user. Secara teknis juga implementasinya sudah sesuai dengan issuenya.
Namun yang mau saya tanyakan adalah ini jadinya tetap tidak menghandle case dimana ketika user role adalah Practitioner, dia kan akan memiliki setidaknya 2 FHIR resources, Patient dan juga Practitioner. Untuk case demikian, apakah jadinya kita tidak akan embed semua resources milik user?
The reason will be displayed to describe this comment to others. Learn more.
Hi Mas @luckyAkbar, thank you for bringing this topic up. I believe it's best to best to embed all user resource IDs in the metadata. I expect the metadata for roles and user ID to be:
Mas @gilanglahat22 please note that for roles Clinic Admin and Researcher, the Person resource is used because there's no dedicated FHIR resource for these roles..
The reason will be displayed to describe this comment to others. Learn more.
this comment is not a change request to the code, just for discussion
mas @lamurian , aku recheck current issue yang akan di solve oleh PR ini dan secara teknis ini adalah function utama yang akan mengambil FHIR resources yang dimiliki oleh user. Secara teknis juga implementasinya sudah sesuai dengan issuenya.
Namun yang mau saya tanyakan adalah ini jadinya tetap tidak menghandle case dimana ketika user role adalah Practitioner, dia kan akan memiliki setidaknya 2 FHIR resources, Patient dan juga Practitioner. Untuk case demikian, apakah jadinya kita tidak akan embed semua resources milik user?
Setuju mas @luckyAkbar, memang harusnya tidak reuse InitializeNewUserFHIRResources karena fokusnya write operation. Sudah saya buatkan function baru khusus untuk read operation:
Function ini hanya query existing FHIR resources berdasarkan SuperTokenUserID, tanpa create resource baru. Sudah menggunakan FindPractitionerByIdentifier, FindPatientByIdentifier, dan PersonFhirClient.Search.
The reason will be displayed to describe this comment to others. Learn more.
Hi Mas @luckyAkbar, thank you for bringing this topic up. I believe it's best to best to embed all user resource IDs in the metadata. I expect the metadata for roles and user ID to be:
Mas @gilanglahat22 please note that for roles Clinic Admin and Researcher, the Person resource is used because there's no dedicated FHIR resource for these roles..
Terima kasih discussionnya mas @luckyAkbar dan mas @lamurian. Untuk sekarang implementasinya masih menggunakan priority-based approach (Practitioner > Patient > Person) untuk menentukan fhirResourceId yang di-embed di access token.
Untuk case user dengan multiple roles (e.g., Practitioner + Patient), saat ini kita hanya embed satu primary resource ID. Jika ke depan ada kebutuhan untuk embed semua resource IDs seperti yang mas @lamurian sarankan:
We reviewed changes in 15fce9a...9ac34c2 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@internal/app/services/core/auth/auth_supertoken_impl.go`:
- Around line 479-503: The code in authUsecase.CreateNewSession uses two
separate checks for fhirErr (if fhirErr != nil and if fhirErr == nil) after
calling uc.getFhirResourceIdForUser; simplify to a single if/else: call
uc.getFhirResourceIdForUser, then if fhirErr != nil log the error and set
accessTokenPayload[supertokenAccessTokenPayloadFhirResourceId] = "" in the if
branch, else set accessTokenPayload[supertokenAccessTokenPayloadFhirResourceId]
= fhirResourceId and log the success; remove the redundant second if and keep
the same logging/messages and keys (accessTokenPayload,
supertokenAccessTokenPayloadFhirResourceId,
supertokenAccessTokenPayloadRolesKey).
In `@internal/app/services/core/users/user_usecase_impl.go`:
- Around line 384-438: LookupUserFHIRResourceIDs currently swallows all errors
from uc.PractitionerFhirClient.FindPractitionerByIdentifier,
uc.PatientFhirClient.FindPatientByIdentifier, and uc.PersonFhirClient.Search and
always returns (output, nil), making callers' err checks (e.g.,
getFhirResourceIdForUser) useless; update LookupUserFHIRResourceIDs to return a
meaningful error when appropriate — e.g., if all three lookups failed (no IDs
found and each call returned a non-nil error) return a consolidated error (or
wrap the first error) so callers can handle failure, otherwise keep returning
(output, nil) when at least one lookup succeeded; reference the function name
LookupUserFHIRResourceIDs and the FHIR client calls
(FindPractitionerByIdentifier, FindPatientByIdentifier, PersonFhirClient.Search)
to locate where to implement this logic.
EnsureAnonymousSession must set constvars.CONTEXT_* keys to match SessionOptional.
This middleware only sets the deprecated local keys (keyRoles, keyUID) but omits constvars.CONTEXT_UID and constvars.CONTEXT_FHIR_RESOURCE_ID. Multiple downstream handlers read from these constvars keys and will receive zero values when this middleware is used, causing bugs. Add:
In `@internal/app/delivery/http/middlewares/session.go`:
- Around line 85-89: The comment above the context.WithValue calls is garbled;
replace it with a clear sentence such as: "Typed context keys
(constvars.ContextKey) — these will replace the deprecated local ContextKey keys
above." Update the comment located near the context.WithValue(...) lines that
set constvars.CONTEXT_FHIR_ROLE, constvars.CONTEXT_UID, and
constvars.CONTEXT_FHIR_RESOURCE_ID so it clearly states that typed constvars
keys are used and will deprecate untyped string context keys.
In `@internal/app/services/core/auth/auth_supertoken_impl.go`:
- Around line 167-177: Remove the five calls to the undefined package-level
function ensureRoleExists() (the bare calls at the start of the list) and keep
only the method calls on the use case receiver uc (uc.ensureRoleExists(...));
specifically delete the calls to
ensureRoleExists(constvars.KonsulinRolePatient),
ensureRoleExists(constvars.KonsulinRoleGuest),
ensureRoleExists(constvars.KonsulinRoleClinicAdmin),
ensureRoleExists(constvars.KonsulinRolePractitioner), and
ensureRoleExists(constvars.KonsulinRoleResearcher) so only
uc.ensureRoleExists(...) and
uc.ensureRoleExists(constvars.KonsulinRoleSuperadmin) remain, relying on the
defined method uc.ensureRoleExists.
244-263: Default userRoles always includes Patient, then appends SuperTokens roles — potential duplicate.
userRoles starts as [Patient] (line 244), then line 262 appends the user's actual roles. If the user already has the Patient role in SuperTokens, the slice becomes [Patient, Patient, ...]. This likely doesn't cause a bug since ToogleByRoles and getFhirResourceIdForUser just scan for membership, but it's worth cleaning up.
Proposed fix
- if userRolesResp.OK != nil {- // override the default roles with the user roles from supertokens- userRoles = append(userRoles, userRolesResp.OK.Roles...)- }+ if userRolesResp.OK != nil && len(userRolesResp.OK.Roles) > 0 {+ // override the default roles with the user roles from supertokens+ userRoles = userRolesResp.OK.Roles+ }
The reason will be displayed to describe this comment to others. Learn more.
untuk block if fhirErr == nil ini juga menurut saya tidak diperlukan, mas, karena sudah pasti value fhirErr adalah nil, kan.
Lebih baik if checking nya itu mengecek fhirResourceId, apakah dia nil atau apakah mengandung value tertentu yang dianggap valid untuk digunakan di accessTokenPayload[supertokenAccessTokenPayloadFhirResourceId]
The reason will be displayed to describe this comment to others. Learn more.
Untuk flow userID == "", sudah saya adjust supaya langsung set payload sebagai Guest dan fhirResourceId empty string, lalu early return ke originalCreateNewSession. Jadi block di bawahnya tidak perlu ikut masuk nested else @luckyAkbar
The reason will be displayed to describe this comment to others. Learn more.
Thanks mas @luckyAkbar , noted. Sudah saya ubah supaya assignment accessTokenPayload[supertokenAccessTokenPayloadFhirResourceId] tidak lagi bergantung ke fhirErr == nil, tapi mengecek value fhirResourceId != "". Jadi kalau lookup sukses tapi resource ID kosong, payload tetap di-set empty string dan tidak menyimpan value yang tidak valid.
The reason will be displayed to describe this comment to others. Learn more.
kemudian untuk bagian ini, sepemahaman saya adalah ketika userID dari supertokens tidak ditemukan, maka payload di accessTokenPayload hanya akan diinisiasi sbg role Guest dan juga resource id nya di set ke empty string.
Supaya untuk mempermudah kodenya di baca, kita bisa pakai gaya penulisan code negative space programming. Karena case userID == "" ini dianggap sebagai error / default behaviour, kita bisa langsung melakukan menambahkan return line (Seperti yang ada di baris 578) untuk langsung skip semua kode yang ada di bawahnya.
Setelah itu, else block yang menempel (line 539) bisa langsung kita hapus karena ketika case userID == "" kode akan langsung exit di block tersebut. Akibatnya kode yang ditulis itu lebih rata ke kiri ketimbang berada di dalam nested if yang dalam.
Hal yang sama juga saya sarankan untuk menghilangkan block else di baris ke 570 - 576 dengan cara lakukan error check yang lebih eksplisit dari hasil operasi function line 540. Gambaran saya seperti ini
rolesResp, err := userroles.GetRolesForUser(tenantId, userID)
if err != nil || roleresp.OK == nil {
// bisa di log dulu kenapa supertokens error
accessTokenPayload[supertokenAccessTokenPayloadRolesKey] = map[string]interface{}{
supertokenAccessTokenPayloadRolesValueKey: []interface{}{constvars.KonsulinRoleGuest},
}
accessTokenPayload[supertokenAccessTokenPayloadFhirResourceId] = ""
return originalCreateSession....
}
// rest of the code here (line 542 - last return line
Menurut saya ini gak mengubah flow execution yang sudah di buat, melainkan hanya melakukan early exit ketika kode mengalami kondisi yang tidak diinginkan / tidak ideal dan sekaligus mengurangi nested kode, mas @gilanglahat22 . Please LMK your response ya mas
The reason will be displayed to describe this comment to others. Learn more.
Thanks mas @luckyAkbar , setuju. Sudah saya adjust supaya case userID == "" langsung dianggap sebagai default/guest flow. Payload di-set ke role Guest dan fhirResourceId empty string, lalu langsung return ke originalCreateNewSession. Dengan begitu nested else besar bisa dihilangkan dan flow di bawah
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer TIP This summary will be updated as you push new changes.
Unify API-key context key reads with the typed constant.
This function and webhook_controller.go read "api_key_auth" as raw strings while other middleware code uses the typed constant ContextAPIKeyAuth. Since the constant is defined in the middleware, inconsistent raw-string readers create a maintenance hazard: future changes to the constant value won't automatically propagate to these locations. Apply the constant in both auth_helper.go and webhook_controller.go for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/app/services/core/webhook/auth_helper.go` around lines 27 - 57, The
code in extractAuthContext (and the reader in webhook_controller.go) uses the
raw string "api_key_auth"; change those to use the typed constant
ContextAPIKeyAuth (the exported constant from the middleware package) so the key
is consistent with the middleware definition. Update the imports in
internal/app/services/core/webhook/auth_helper.go and webhook_controller.go to
include the middleware package (or the package that defines ContextAPIKeyAuth),
replace ctx.Value("api_key_auth") with ctx.Value(middleware.ContextAPIKeyAuth)
in extractAuthContext, and make the analogous replacement where
webhook_controller.go reads the API-key auth flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/app/delivery/http/middlewares/session.go`:
- Around line 81-88: EnsureAnonymousSession is only seeding the deprecated local
context keys (keyUID, keyRoles, keyFHIRResourceId), causing handlers that expect
the typed contract from SessionOptional to miss auth info; update
EnsureAnonymousSession to also set the typed keys constvars.CONTEXT_UID,
constvars.CONTEXT_FHIR_ROLE, and constvars.CONTEXT_FHIR_RESOURCE_ID on the
request context (in addition to the existing local keys) so anonymous flows
publish the same context contract as SessionOptional; locate the seeding logic
in EnsureAnonymousSession and add context.WithValue calls that mirror the ones
in SessionOptional using those constvars constants.
In `@internal/app/services/core/auth/auth_supertoken_impl.go`:
- Around line 237-259: The code currently seeds userRoles with Patient and then
appends fetched roles in SupertokenCreateCode, which forces Patient onto
existing users; change the logic in the block that calls
userroles.GetRolesForUser (and where userRoles is set) to replace the default
when a successful response returns roles: set userRoles = userRolesResp.OK.Roles
instead of appending; optionally keep the default Patient only when the fetch
succeeds but returns zero roles (i.e., if userRolesResp.OK.Roles is empty, leave
userRoles as []string{constvars.KonsulinRolePatient}); ensure this change is
applied around the userRecord != nil branch handling in auth_supertoken_impl.go.
- Around line 168-188: ensureRoleExists currently swallows errors, so
InitializeSupertoken may report success even if critical roles weren't created;
change ensureRoleExists(role string) to return error (or bool) and return the
userroles.CreateNewRoleOrAddPermissions error when it fails, and update
InitializeSupertoken to check each ensureRoleExists call (for
KonsulinRolePatient, KonsulinRoleGuest, KonsulinRoleClinicAdmin,
KonsulinRolePractitioner, KonsulinRoleResearcher, KonsulinRoleSuperadmin) and
return an error if any role creation fails; preserve/info-log the "role already
exists" path but propagate failures up so InitializeSupertoken returns a non-nil
error instead of logging only.
---
Outside diff comments:
In `@internal/app/services/core/webhook/auth_helper.go`:
- Around line 27-57: The code in extractAuthContext (and the reader in
webhook_controller.go) uses the raw string "api_key_auth"; change those to use
the typed constant ContextAPIKeyAuth (the exported constant from the middleware
package) so the key is consistent with the middleware definition. Update the
imports in internal/app/services/core/webhook/auth_helper.go and
webhook_controller.go to include the middleware package (or the package that
defines ContextAPIKeyAuth), replace ctx.Value("api_key_auth") with
ctx.Value(middleware.ContextAPIKeyAuth) in extractAuthContext, and make the
analogous replacement where webhook_controller.go reads the API-key auth flag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5951d6c0-7133-48b3-be2d-eb0bc7bd6930
📥 Commits
Reviewing files that changed from the base of the PR and between adb496c and 5eb0910.
The reason will be displayed to describe this comment to others. Learn more.
Method receiver 'uc' is not referenced in method's body, consider removing it
Methods with unused receivers can be a symptom of unfinished refactoring or a bug. To keep
the same method signature, omit the receiver name or '_' as it is unused.
The reason will be displayed to describe this comment to others. Learn more.
SUGGESTION: Extra tab indentation before next.ServeHTTP call - should align with other statements in the function body (single tab instead of double tab).
The reason will be displayed to describe this comment to others. Learn more.
Unused method receiver '_', consider removing it
Methods with unused receivers can be a symptom of unfinished refactoring or a bug. To keep
the same method signature, omit the receiver name or '_' as it is unused.
The reason will be displayed to describe this comment to others. Learn more.
receiver name should not be an underscore, omit the name if it is unused
The name of a method's receiver should be a reflection of its
identity; often a one or two letter abbreviation of its type
suffices (such as "c" or "cl" for "Client").
This file contains hidden or 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
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.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Implementation Solutions
This document describes the solution implementations for the following issues in Konsulin API.
Issue 1: Enrich SuperTokens Profile Metadata with Roles and FHIR Resource IDs
Problem
SuperTokens profiles did not provide sufficient metadata for direct authorization checks. Whenever resource ownership validation was needed, the system had to perform additional queries to the database or FHIR server, causing overhead and latency.
Solution
Profile metadata was enriched with:
Patient/{ID},Practitioner/{ID}, orPerson/{ID}based on the user’s primary roleThis metadata is embedded in the SuperTokens access token payload so it can be read without extra queries.
Implementation Details
1. Access Token Payload Enrichment
File:
internal/app/services/core/auth/auth_supertoken_impl.goNew constant:
New helper:
getFhirResourceIdForUser(ctx, userID, roles) (string, error)It derives the FHIR resource ID from the user’s roles with this priority:
Practitioner/{ID}(highest)Patient/{ID}Person/{ID}Session creation change: The
CreateNewSessionoverride addsfhirResourceIdto the access token payload by callinggetFhirResourceIdForUser()and settingaccessTokenPayload[supertokenAccessTokenPayloadFhirResourceId].2. Middleware Enhancement
File:
internal/app/delivery/http/middlewares/session.goNew constants:
keyFHIRResourceId,supertokenAccessTokenPayloadFhirResourceIdSessionOptional middleware: Reads
fhirResourceIdfrom the access token payload and stores it in the request context (keyFHIRResourceId,CONTEXT_FHIR_RESOURCE_ID).3. Context Constant
File:
internal/pkg/constvars/internal_app.goAccess Token Payload Shape
{ "st-role": { "v": ["Patient", "Practitioner"] }, "fhirResourceId": "Practitioner/abc123" }Benefits
Usage (middleware or handler)
Files Modified
internal/app/services/core/auth/auth_supertoken_impl.go– constant,getFhirResourceIdForUser(),CreateNewSessionoverrideinternal/app/delivery/http/middlewares/session.go– constants,SessionOptionalreading/storingfhirResourceIdinternal/pkg/constvars/internal_app.go–CONTEXT_FHIR_RESOURCE_IDTesting
getFhirResourceIdForUser()for different role combinations and priority (Practitioner > Patient > Person).Issue 2: Use FHIR Bundle for Batch PractitionerRole Availability Updates
Problem
The PractitionerAvailabilityEditor updated multiple PractitionerRole resources one-by-one in a loop. If one update failed mid-batch, earlier updates were already persisted, leading to an inconsistent state that was hard to recover.
Solution
The availability update flow was refactored to use a FHIR Bundle transaction: the client sends one request with all PractitionerRole updates in a Bundle, and the backend processes it atomically (all-or-nothing). The backend already had the infrastructure to support FHIR Bundle transactions.
Implementation Details (Backend Support)
1. Bundle FHIR Client
File:
internal/app/services/fhir_spark/bundle/bundle_fhir_impl.go2. Middleware Auth – Bundle Validation
File:
internal/app/delivery/http/middlewares/auth.goAuthusesscanBundle()to validate every entry in the Bundle (resourceType, RBAC, resource ownership, structure). If any entry fails, the whole request is rejected.3. Middleware Bridge – Bundle Proxy
File:
internal/app/delivery/http/middlewares/proxy.goBridgeforwards the request (including Bundle) to the FHIR server withContent-Type: application/fhir+json.4. Router
File:
internal/app/delivery/http/routers/router.goFHIR Bundle Transaction Shape
{ "resourceType": "Bundle", "type": "transaction", "entry": [ { "request": { "method": "PUT", "url": "PractitionerRole/{id1}" }, "resource": { "resourceType": "PractitionerRole", "id": "{id1}", "availableTime": [ { "daysOfWeek": ["mon", "tue"], "availableStartTime": "09:00:00", "availableEndTime": "17:00:00" } ] } } ] }Request and Validation Flow
POST /fhir(Bundle) → Auth → Bridge → FHIR serverscanBundle()on each entry (RBAC + ownership). If all pass → Bridge forwards; otherwise → error.Benefits
Flow Comparison
Before (sequential)
Update #1 ✅ → #2 ✅ → #3 ❌ → inconsistent state; user must fix manually.
After (Bundle)
Single Bundle → all succeed or all fail → consistent state; user can retry the whole operation.
Error Handling
If any entry fails validation or the FHIR server rejects the transaction, the backend can return an OperationOutcome; the FHIR server rolls back the whole transaction. The frontend gets a single error and can retry the full batch.
Files Involved
internal/app/services/fhir_spark/bundle/bundle_fhir_impl.go– Bundle clientinternal/app/delivery/http/middlewares/auth.go–scanBundle()internal/app/delivery/http/middlewares/proxy.go– Bridgeinternal/app/delivery/http/routers/router.go–/fhirrouteinternal/pkg/fhir_dto/bundle.go– Bundle DTOFrontend implementation:
konsulin-app(schedule API and practitioner-availability-editor).Testing
scanBundle()with different Bundle structures and error cases.References
Summary by CodeRabbit
New Features
Refactor