Skip to content

Embedded esignet-service with new thunderidengine - #2028

Merged
anushasunkada merged 3 commits into
mosip:develop-gofrom
anushasunkada:engine-int
Jun 23, 2026
Merged

anushasunkada merged 3 commits into
mosip:develop-gofrom
anushasunkada:engine-int

Conversation

@anushasunkada

@anushasunkada anushasunkada commented Jun 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

Release Notes

  • New Features
    • Added modular authentication providers (mock, MOSIP IDA, SunbirdRC).
    • Added client-management token validation with optional scope enforcement.
    • Client management now returns public_key and enc_public_key.
  • Configuration Changes
    • Renamed key environment variables: ISSUER_URL → MOSIP_ESIGNET_HOST; POSTGRES_* → DATABASE_*/DB_DBUSER_PASSWORD.
    • Updated eSignet and UI OIDC/gate settings and related defaults (including container port shift to 8088).
  • Bug Fixes & Improvements
    • Improved client-management JSON decode error details.
  • Chores
    • Updated README, Docker/run scripts, Postman collections, and smoke-test flow guidance.

Signed-off-by: anushasunkada <anushasunkada@gmail.com>
@coderabbitai

coderabbitai Bot commented Jun 22, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@anushasunkada, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 43 minutes and 41 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 15af27bb-4004-4ec1-bde5-3d9375a84216

📥 Commits

Reviewing files that changed from the base of the PR and between a5e4b55 and 11da1c2.

📒 Files selected for processing (1)
  • esignet-service/.env.example

Walkthrough

The esignet-service is refactored to move ThunderID engine integration from the legacy internal/host and internal/catalog packages into a new internal/engine sub-package tree with dedicated mosip, sunbird, and mock provider packages. The main.go startup is rewired to load a unified AppConfig and conditionally wire all engine components. Environment variables are renamed (ISSUER_URL→MOSIP_ESIGNET_HOST, POSTGRES_*→DATABASE_*), the default port changes to 8088, declarative YAML fixtures adopt camelCase keys, executor names change to CredentialsAuthExecutor, and Postman collections are extended with private_key_jwt support and a new client-management folder.

Changes

esignet-service Engine Refactor & Env Variable Rename

Layer / File(s) Summary
App & infra config contracts
esignet-service/internal/config/app.go, internal/config/db.go, internal/config/redis.go, go.mod
Introduces AppConfig/LoadAppConfig() to consolidate port, issuer, data directory, DB, and Redis config. Renames LoadDB/LoadRedis to unexported loadDB/loadRedis and switches DB env vars to DATABASE_HOST/PORT/NAME/USERNAME + DB_DBUSER_PASSWORD. Removes ensurePostgresSSLMode helper and defaultRedisDB constant. Updates module dependencies to include direct requirements for jwt/v5, lib/pq, go-redis/v9, testify, thunderid, and go-pkcs12.
Client-mgmt config & response
esignet-service/internal/clientmgmt/config.go, internal/clientmgmt/service.go, internal/clientmgmt/handler.go
Adds clientmgmt.Config, LoadConfig(), and ScopeEnforcementEnabled() for optional JWKS-backed scope enforcement driven by CLIENT_MGMT_ISSUER_URL and CLIENT_MGMT_JWKS_ENDPOINT. Extends ClientResponse with public_key and enc_public_key fields. Updates handler to accept nil middleware and surface real JSON decode errors.
engine.Config & BuildThunderConfig
esignet-service/internal/engine/config.go
New Config struct and LoadConfig() for provider/flow/theme IDs. BuildThunderConfig() loads Thunder engine config, enforces crypto encryption key, enables declarative stores across multiple domains, parses OIDC_UI_* gate-client parameters, and parses OAuth lifetime env vars (auth code, PAR, access token) with validation. PKIPaths() resolves signing cert/key paths.
MOSIP authn provider & OTP executor
esignet-service/internal/engine/mosip/config.go, mosip/mosip_authn.go, mosip/mosip_otp_executor.go
Moves MOSIP config/authn into the mosip package. Renames env vars to MOSIP_ESIGNET_* and IDA_AUTHENTICATOR_ENV. Injects clientmgmt.Service into NewMosipAuthnProvider for RP ID resolution. Adds getApplicationAndClientID helper. Updates GetAttributes to use requested.Names. Updates GenerateTransactionID signature. Adds NewMosipOtpExecutor with full execute/prerequisite/metadata logic including buildAuthMetadata to construct runtime metadata from authorization/client identifiers.
SunbirdRC authn provider
esignet-service/internal/engine/sunbird/config.go, sunbird/sunbird_authn.go
Moves Sunbird config/authn into the sunbird package, replacing SunbirdAuthn/LoadSunbirdAuthn() with Config/LoadConfig(), and removing internal/config dependency. Adds envOrDefault and trimTrailingSlash helpers.
Mock authn provider
esignet-service/internal/engine/mock/config.go, mock/mock_authn.go
Adds the mock package with OTPAuthnProvider interface and mockAuthnProvider returning fixed successful results for Authenticate, GetAttributes, and SendOTP.
Actor, authz, consent providers & authn factory
esignet-service/internal/engine/actors.go, authz.go, consent.go, authn_factory.go, executors.go
New engine-package host-layer implementations: DB-backed actorProvider, permissive roleProvider, no-op consentEnforcer, provider-keyed NewAuthnProviderFromConfig factory, and CustomExecutors. Removes legacy internal/host/* and internal/catalog and internal/store/redis_store.
main.go startup refactor
esignet-service/cmd/esignet/main.go
Rewrites startup to load AppConfig, conditionally wire client-mgmt scope middleware, build all engine providers, and initialize Thunder engine via builder-style thunderidengine.New(...) with deferred shutdown.
Declarative YAML fixture updates
esignet-service/data/config/resources/*, data/deployment.yaml
Renames all YAML keys to camelCase (ouId, authFlowId, allowSelfRegistration, systemAttributes), changes executor names to CredentialsAuthExecutor across all flows, adds deployment.yaml enabling declarative stores, and removes repository-level application/agent YAML files.
Dockerfile & make.sh wiring
esignet-service/Dockerfile, make.sh
Renames ISSUER_URL→MOSIP_ESIGNET_HOST ARG/ENV, changes exposed port to 8088, adds backward-compat shim in make.sh, switches AUTHN_PROVIDER default to mosip, and updates run/smoke/docker-run targets and help text.
Env example, Postman & JWT key script
esignet-service/.env.example, postman/embedder-local.environment.json, postman/embedder-positive-flow.json, scripts/generate-smoke-jwt-client-key.sh
Updates .env.example with new env var names; adds private_key_jwt prerequest script and folder 4 (client management) to Postman collection; adds JWK/kid environment variables; rewrites key-gen script to produce private JWK, public JWKS, and kid.
README
esignet-service/README.md
Updates layout tree, env var tables (DATABASE_, MOSIP_ESIGNET_, client-mgmt, provider config), Docker run examples, and Postman folder descriptions to match renamed env vars and restructured packages.

Sequence Diagram(s)

sequenceDiagram
  participant main
  participant config as config.AppConfig
  participant clientmgmt as clientmgmt.Config
  participant engine as engine.BuildThunderConfig
  participant providers as engine.NewAuthnProviderFromConfig
  participant thunder as thunderidengine.New
  participant mux as http.ServeMux

  main->>config: LoadAppConfig()
  config-->>main: port, issuer, DB, Redis
  main->>config: DB.Open(), Redis.Open()
  main->>clientmgmt: LoadConfig()
  clientmgmt-->>main: config
  alt ScopeEnforcementEnabled
    main->>main: build JWKS cache middleware
  else
    main->>main: log warning, nil middleware
  end
  main->>engine: BuildThunderConfig(appCfg)
  engine-->>main: thunderCfg, enabledExecutors
  main->>providers: NewAuthnProviderFromConfig(provider, clientSvc)
  providers-->>main: authnProvider (mosip/sunbird/mock)
  main->>thunder: New(thunderCfg, actorProvider, authnProvider, roleProvider, consentEnforcer, ...)
  thunder-->>main: eng
  main->>eng: RegisterRoutes(mux)
  main->>mux: ListenAndServe(:port)
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related issues

Possibly related PRs

  • mosip/esignet#1871: This PR removes/replaces the internal/catalog and internal/host packages that PR #1871 introduced, rewiring all actor/authn/consent integrations into the new internal/engine sub-packages.
  • mosip/esignet#1989: Both PRs modify esignet-service/internal/clientmgmt — adding/updating scope enforcement config, handler wiring, and service response fields in overlapping areas.
  • mosip/esignet#2014: Both PRs touch esignet-service/scripts/generate-smoke-jwt-client-key.sh and the private_key_jwt smoke-client tooling, with this PR extending it to produce full private JWK and kid output.

Suggested reviewers

  • sacrana0
  • zesu22
  • jayesh12234

Poem

🐇 Hop hop, the catalog's gone away,
New engine packages bloom today!
ISSUER_URL hops off to retire,
MOSIP_ESIGNET_HOST climbs higher~
Port 8088 is where we now play,
Mock, MOSIP, Sunbird — all here to stay! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 55.26% 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 'Embedded esignet-service with new thunderidengine' directly describes the main architectural change: integrating the esignet-service with a refactored Thunder engine, which is the primary focus of the changeset.
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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 18

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
esignet-service/internal/config/db.go (1)

45-67: ⚠️ Potential issue | 🟠 Major

Add sslmode=disable normalization to POSTGRES_URL to match manual DSN construction behavior.

Line 46 uses POSTGRES_URL verbatim, but the code explicitly sets sslmode=disable when manually constructing a DSN (lines 51–58). Since lib/pq defaults to sslmode=require, a bare POSTGRES_URL without sslmode will fail against common non-TLS local Postgres setups, creating an inconsistency. The README.md example shows sslmode=disable is expected in the URL.

Suggested patch
 func loadDB() DB {
 	dsn := os.Getenv("POSTGRES_URL")
+	if dsn != "" &&
+		(strings.HasPrefix(dsn, "postgres://") || strings.HasPrefix(dsn, "postgresql://")) &&
+		!strings.Contains(strings.ToLower(dsn), "sslmode=") {
+		sep := "?"
+		if strings.Contains(dsn, "?") {
+			sep = "&"
+		}
+		dsn += sep + "sslmode=disable"
+	}
 	if dsn == "" {
 		host := envOrDefault("DATABASE_HOST", "localhost")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@esignet-service/internal/config/db.go` around lines 45 - 67, In the loadDB
function, when the POSTGRES_URL environment variable is set, it is used directly
without normalizing the sslmode parameter. However, the manually constructed DSN
explicitly sets sslmode=disable to ensure consistency with non-TLS local
Postgres setups. Normalize the POSTGRES_URL by checking if it already contains
an sslmode parameter, and if not, append sslmode=disable to maintain consistency
with the manual DSN construction behavior shown in the password-conditional
branching.
esignet-service/README.md (1)

72-90: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Binary run example needs PORT export to match the shown host/issuer.

This block sets MOSIP_ESIGNET_HOST to :8080 but does not set PORT; with current defaults the binary listens on 8088, which conflicts with the shown addr=:8080 flow.

Suggested fix
 ./make.sh keys && ./make.sh build
 
+export PORT=8080
 export MOSIP_ESIGNET_HOST=http://127.0.0.1:8080
 export DATABASE_HOST=localhost
 export DATABASE_USERNAME=esignet
 export DB_DBUSER_PASSWORD=secret
 export DATABASE_NAME=mosip_esignet
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@esignet-service/README.md` around lines 72 - 90, The binary run example in
the README.md exports MOSIP_ESIGNET_HOST set to port 8080 but does not set the
PORT environment variable, causing the server to listen on the default port 8088
instead of 8080 as shown in the expected startup log. Add an export statement
for PORT=8080 in the environment variables section before the binary execution
command to ensure the server actually listens on the port specified in
MOSIP_ESIGNET_HOST and matches the addr=:8080 shown in the startup log output.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@esignet-service/.env.example`:
- Around line 125-128: Replace hyphens with underscores in the environment
variable names on lines 126-128 of the `.env.example` file where the IDA
authenticator override keys are documented. Specifically, update
MOSIP_ESIGNET_AUTHENTICATOR_IDA_SEND-OTP-URL to use SEND_OTP_URL,
MOSIP_ESIGNET_AUTHENTICATOR_IDA_KYC-AUTH-URL to use KYC_AUTH_URL, and
MOSIP_ESIGNET_AUTHENTICATOR_IDA_KYC-EXCHANGE-URL to use KYC_EXCHANGE_URL instead
of hyphens, since bash cannot source variable assignments with hyphens in their
names when make.sh sources this file. After making these changes, verify that
the same variable names are consistently documented in the make.sh help text and
README.md to ensure the documented override path is functional and consistent
across all documentation.

In `@esignet-service/data/deployment.yaml`:
- Around line 19-21: The crypto.encryption.key field contains a hardcoded
encryption key value that is exposed in source control, creating a security
vulnerability. Replace the hardcoded key string with a reference to an
environment variable called CRYPTO_ENCRYPTION_KEY. In the deployment.yaml file,
update the encryption key value to use the environment variable reference syntax
(typically ${CRYPTO_ENCRYPTION_KEY} or $CRYPTO_ENCRYPTION_KEY depending on your
YAML processor), and ensure this environment variable is configured in your
environment or secrets management system rather than committed to the
repository.

In `@esignet-service/go.mod`:
- Line 66: The replace directive for github.com/thunder-id/thunderid in the
go.mod file currently points to a personal fork
(github.com/anushasunkada/thunder/backend) instead of the official org-owned
repository. Update the replace statement on line 66 to point to the official
github.com/thunder-id/thunderid source repository instead, ensuring production
releases depend on the authoritative source with proper provenance guarantees.
If an environment variable override mechanism exists via THUNDER_MODULE, ensure
it is properly documented and enforced in all release pipelines rather than
relying on a personal fork as the default checked-in dependency.

In `@esignet-service/internal/clientmgmt/config.go`:
- Around line 16-18: The JWKSEndpoint field has a documented default behavior
that should derive its value from MOSIP_ESIGNET_HOST with the path
/.well-known/jwks.json when unset, but this default is not computed in the
LoadConfig() function. This causes ScopeEnforcementEnabled() to silently fail
when only the issuer is configured, leaving routes unprotected. Implement the
documented default by modifying LoadConfig() to check if JWKSEndpoint is empty
and, if so, construct it from the MOSIP_ESIGNET_HOST value by appending the path
/.well-known/jwks.json to it. Apply this same logic to all locations where
JWKSEndpoint is handled (including lines 35-37 and 56-60 as indicated).
- Around line 43-52: The JWKSCacheTTL field in the Config struct initialization
is converting the ttlSecs integer directly to time.Duration without applying the
proper time unit multiplier, causing it to be interpreted as nanoseconds instead
of seconds. Fix this by multiplying the time.Duration(ttlSecs) cast by
time.Second to properly convert the duration value from seconds to nanoseconds,
which is what time.Duration expects.

In `@esignet-service/internal/clientmgmt/service.go`:
- Around line 66-67: The EncPublicKey field is declared in the struct with a
JSON tag enc_public_key,omitempty but is never populated in the toResponse()
method. Locate the toResponse() method and find where PublicKey is being
assigned to the response object, then add a corresponding assignment for
EncPublicKey with the appropriate encrypted public key value from the source
object to ensure the new field is properly populated in the API response.

In `@esignet-service/internal/engine/authn_factory.go`:
- Around line 34-36: The error message in the default case of the switch
statement does not include "mock" as a valid authentication provider option,
even though "mock" is handled as a valid case earlier in the switch statement.
Update the error message in the fmt.Errorf call to include "mock" alongside
"mosip" and "sunbird" so that the error message accurately reflects all
supported provider options.

In `@esignet-service/internal/engine/config.go`:
- Around line 90-95: After successfully converting gateClientPort to an integer
using strconv.Atoi, add validation to ensure gateClientPortInt is within the
valid port range of 1 to 65535. If the port value is outside this range, return
an error with a descriptive message similar to the existing error format (e.g.,
"invalid OIDC_UI_PORT: port must be between 1 and 65535"). This validation
should occur before assigning the value to cfg.GateClient.Port to prevent
invalid port configurations from reaching runtime code.
- Around line 132-139: The filepath.Rel() function requires both arguments to
have consistent path types (both absolute or both relative), but the current
code mixes an absolute dataDir with potentially relative cert and key paths.
Modify the code to ensure consistent path types before calling filepath.Rel() by
converting the relative cert and key paths to absolute paths when necessary.
Before calling filepath.Rel(certRel, dataDir) and filepath.Rel(keyRel, dataDir),
check if cert and key are relative paths, and if so, join them with dataDir to
make them absolute before passing to filepath.Rel().

In `@esignet-service/internal/engine/consent.go`:
- Around line 23-24: The log.Println calls in the ResolveConsent function are
logging sensitive information including user identifiers (userID, ouID),
application details (appID, appName), permissions, and runtime metadata which
should not be exposed in logs to prevent PII and secret leakage. Remove or
replace these log statements with a generic message that does not include any of
the sensitive parameters. This same fix also applies to the similar logging
statement at lines 33-34 in the same file.
- Around line 13-15: The `NewConsentEnforcer()` function returns a
`consentEnforcer` struct that has no-op implementations which always return nil
data and nil errors, preventing actual consent enforcement and persistence.
Locate the method implementations of the `consentEnforcer` type (referenced at
lines 25-26 and 35-35) and replace these no-op implementations with proper logic
that actually enforces consent rules, resolves consent data, and persists the
consent information. Ensure that the methods no longer return nil values in both
success and error paths but instead contain the actual enforcement and
persistence logic required by the thunderidengine.ConsentEnforcer interface.

In `@esignet-service/internal/engine/mock/mock_authn.go`:
- Around line 29-31: The SendOTP method in the mockAuthnProvider struct is
logging raw authentication payloads including identifiers and metadata, which
exposes sensitive data. Remove the log.Println call or refactor it to avoid
logging the identifiers and metadata parameters directly. Apply the same fix to
the other mock provider methods that have similar logging issues at lines 38-40
and 48-50, ensuring no raw auth payloads are logged in any of the mock authn
provider methods.
- Line 42: The unsafe type assertion on the identifiers map in the Authenticate
function will panic if the username key is missing or contains a non-string
value. Replace the direct type assertion `identifiers["username"].(string)` with
a safe type assertion check that verifies both the key existence and type. If
the username is missing or not a string, return an appropriate typed error
instead of allowing the panic to occur.

In `@esignet-service/internal/engine/mosip/mosip_authn.go`:
- Around line 177-178: In the GetAttributes function, the debug log statement
references requested.Names without first checking if the requested parameter is
nil, which will cause a panic if requested is nil. Add a nil guard check before
accessing requested.Names in the applog.Any call, either by conditionally
logging the attribute based on whether requested is not nil, or by providing a
safe fallback value when requested is nil.
- Around line 241-251: The getApplicationAndClientID function silently returns
empty strings when client service lookup fails, which masks underlying
dependency or configuration problems and causes downstream issues with blank
values. Add a nil check for p.clientSvc before calling GetClient, and when the
GetClient call returns an error, log the error appropriately instead of silently
returning empty strings to ensure dependency/configuration problems surface
immediately rather than causing cascading failures with blank relyingPartyID and
clientID values.

In `@esignet-service/internal/engine/mosip/mosip_otp_executor.go`:
- Around line 97-102: The code has two issues:
ctx.RuntimeData["ext_TransactionID"] assignment will panic if ctx.RuntimeData is
nil, and the code unconditionally overwrites any existing ext_TransactionID
value, breaking upstream transaction correlation. Fix this by first checking if
ctx.RuntimeData is nil and initializing it as an empty map if needed, then only
setting ctx.RuntimeData["ext_TransactionID"] to the generated mosipTransactionID
if the ext_TransactionID key does not already exist in ctx.RuntimeData. This
preserves inbound transaction IDs while providing a fallback generated value
when needed.

In `@esignet-service/make.sh`:
- Around line 171-176: In the target_docker_run function, update the docker run
command's port mapping to reflect the new internal container port. Change the
port mapping from 8080 to 8088 in the -p flag so that traffic correctly reaches
the service that now listens on the new port instead of the old one.

In `@esignet-service/README.md`:
- Around line 313-318: The Docker port mapping in the README example is
incorrect because the container now listens on port 8088, not 8080. Update the
`-p` flag in the docker run command to map the host port (8080) to the correct
container listening port (8088) so the service is accessible through the
documented command.

---

Outside diff comments:
In `@esignet-service/internal/config/db.go`:
- Around line 45-67: In the loadDB function, when the POSTGRES_URL environment
variable is set, it is used directly without normalizing the sslmode parameter.
However, the manually constructed DSN explicitly sets sslmode=disable to ensure
consistency with non-TLS local Postgres setups. Normalize the POSTGRES_URL by
checking if it already contains an sslmode parameter, and if not, append
sslmode=disable to maintain consistency with the manual DSN construction
behavior shown in the password-conditional branching.

In `@esignet-service/README.md`:
- Around line 72-90: The binary run example in the README.md exports
MOSIP_ESIGNET_HOST set to port 8080 but does not set the PORT environment
variable, causing the server to listen on the default port 8088 instead of 8080
as shown in the expected startup log. Add an export statement for PORT=8080 in
the environment variables section before the binary execution command to ensure
the server actually listens on the port specified in MOSIP_ESIGNET_HOST and
matches the addr=:8080 shown in the startup log output.
🪄 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: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 994ef298-fc85-41aa-993e-bbd115aa9835

📥 Commits

Reviewing files that changed from the base of the PR and between d701aad and 0821ca2.

⛔ Files ignored due to path filters (1)
  • esignet-service/go.sum is excluded by !**/*.sum
📒 Files selected for processing (74)
  • esignet-service/.env.example
  • esignet-service/Dockerfile
  • esignet-service/README.md
  • esignet-service/cmd/esignet/main.go
  • esignet-service/cmd/esignet/main_test.go
  • esignet-service/data/config/resources/agents/agent-declarative-1.yaml
  • esignet-service/data/config/resources/flows/flow-declarative-1.yaml
  • esignet-service/data/config/resources/flows/flow-declarative-mosip-otp-1.yaml
  • esignet-service/data/config/resources/flows/flow-declarative-multi-flow-1.yaml
  • esignet-service/data/config/resources/flows/flow-declarative-recovery-1.yaml
  • esignet-service/data/config/resources/flows/flow-declarative-registration-1.yaml
  • esignet-service/data/config/resources/flows/flow-declarative-sunbird-1.yaml
  • esignet-service/data/config/resources/identity_providers/idp-declarative-1.yaml
  • esignet-service/data/config/resources/layouts/layout-declarative-1.yaml
  • esignet-service/data/config/resources/organization_units/ou-declarative-1.yaml
  • esignet-service/data/config/resources/resource_servers/resource-declarative-1.yaml
  • esignet-service/data/config/resources/roles/role-declarative-1.yaml
  • esignet-service/data/config/resources/themes/theme-declarative-1.yaml
  • esignet-service/data/config/resources/user_types/usertype-declarative-1.yaml
  • esignet-service/data/config/resources/users/user-declarative-1.yaml
  • esignet-service/data/deployment.yaml
  • esignet-service/data/repository/resources/agents/agent-declarative-confidential.yaml
  • esignet-service/data/repository/resources/applications/app-declarative-1.yaml
  • esignet-service/data/repository/resources/applications/app-declarative-1otp.yaml
  • esignet-service/data/repository/resources/applications/app-declarative-2.yaml
  • esignet-service/data/repository/resources/applications/app-declarative-confidential.yaml
  • esignet-service/data/repository/resources/applications/app-declarative-jwt-client.yaml
  • esignet-service/data/repository/resources/applications/app-declarative-sunbird.yaml
  • esignet-service/go.mod
  • esignet-service/internal/catalog/catalog.go
  • esignet-service/internal/catalog/catalog_test.go
  • esignet-service/internal/catalog/flow_yaml_test.go
  • esignet-service/internal/clientmgmt/config.go
  • esignet-service/internal/clientmgmt/handler.go
  • esignet-service/internal/clientmgmt/service.go
  • esignet-service/internal/config/app.go
  • esignet-service/internal/config/authn.go
  • esignet-service/internal/config/authn_test.go
  • esignet-service/internal/config/clientmgmt.go
  • esignet-service/internal/config/db.go
  • esignet-service/internal/config/db_test.go
  • esignet-service/internal/config/engine.go
  • esignet-service/internal/config/engine_test.go
  • esignet-service/internal/config/mosip_test.go
  • esignet-service/internal/config/redis.go
  • esignet-service/internal/config/sunbird_test.go
  • esignet-service/internal/engine/actors.go
  • esignet-service/internal/engine/authn_factory.go
  • esignet-service/internal/engine/authz.go
  • esignet-service/internal/engine/config.go
  • esignet-service/internal/engine/consent.go
  • esignet-service/internal/engine/executors.go
  • esignet-service/internal/engine/mock/config.go
  • esignet-service/internal/engine/mock/mock_authn.go
  • esignet-service/internal/engine/mosip/config.go
  • esignet-service/internal/engine/mosip/mosip_authn.go
  • esignet-service/internal/engine/mosip/mosip_otp_executor.go
  • esignet-service/internal/engine/sunbird/config.go
  • esignet-service/internal/engine/sunbird/sunbird_authn.go
  • esignet-service/internal/host/actors.go
  • esignet-service/internal/host/authn_factory.go
  • esignet-service/internal/host/authn_factory_test.go
  • esignet-service/internal/host/authz.go
  • esignet-service/internal/host/catalog_authn.go
  • esignet-service/internal/host/consent.go
  • esignet-service/internal/host/executors.go
  • esignet-service/internal/host/mosip_otp_executor.go
  • esignet-service/internal/host/sunbird_authn_test.go
  • esignet-service/internal/store/redis_store.go
  • esignet-service/internal/store/redis_store_test.go
  • esignet-service/make.sh
  • esignet-service/postman/embedder-local.environment.json
  • esignet-service/postman/embedder-positive-flow.json
  • esignet-service/scripts/generate-smoke-jwt-client-key.sh
💤 Files with no reviewable changes (30)
  • esignet-service/internal/catalog/catalog_test.go
  • esignet-service/data/repository/resources/applications/app-declarative-2.yaml
  • esignet-service/data/repository/resources/applications/app-declarative-confidential.yaml
  • esignet-service/internal/host/executors.go
  • esignet-service/internal/host/authn_factory.go
  • esignet-service/internal/catalog/flow_yaml_test.go
  • esignet-service/data/repository/resources/applications/app-declarative-jwt-client.yaml
  • esignet-service/data/repository/resources/applications/app-declarative-1.yaml
  • esignet-service/internal/config/db_test.go
  • esignet-service/internal/config/authn.go
  • esignet-service/cmd/esignet/main_test.go
  • esignet-service/data/repository/resources/applications/app-declarative-sunbird.yaml
  • esignet-service/internal/config/engine.go
  • esignet-service/internal/config/clientmgmt.go
  • esignet-service/internal/host/authn_factory_test.go
  • esignet-service/internal/config/sunbird_test.go
  • esignet-service/data/repository/resources/agents/agent-declarative-confidential.yaml
  • esignet-service/internal/host/mosip_otp_executor.go
  • esignet-service/internal/host/sunbird_authn_test.go
  • esignet-service/internal/config/engine_test.go
  • esignet-service/internal/host/catalog_authn.go
  • esignet-service/internal/config/mosip_test.go
  • esignet-service/internal/catalog/catalog.go
  • esignet-service/data/repository/resources/applications/app-declarative-1otp.yaml
  • esignet-service/internal/host/actors.go
  • esignet-service/internal/store/redis_store.go
  • esignet-service/internal/host/consent.go
  • esignet-service/internal/config/authn_test.go
  • esignet-service/internal/host/authz.go
  • esignet-service/internal/store/redis_store_test.go

Comment thread esignet-service/.env.example Outdated
Comment thread esignet-service/data/deployment.yaml Outdated
Comment thread esignet-service/go.mod
Comment thread esignet-service/internal/clientmgmt/config.go
Comment thread esignet-service/internal/clientmgmt/config.go
Comment thread esignet-service/internal/engine/mosip/mosip_authn.go Outdated
Comment thread esignet-service/internal/engine/mosip/mosip_authn.go Outdated
Comment thread esignet-service/internal/engine/mosip/mosip_otp_executor.go Outdated
Comment thread esignet-service/make.sh
Comment thread esignet-service/README.md Outdated
Signed-off-by: anushasunkada <anushasunkada@gmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@esignet-service/.env.example`:
- Around line 24-25: The CRYPTO_ENCRYPTION_KEY variable in the .env.example file
contains a hardcoded encryption key value that should not be committed as it
weakens security across environments. Replace the actual key value with a
placeholder string that demonstrates the expected format (64-character hex
string) without providing a real key that users might unknowingly reuse across
deployments. Keep the descriptive comment about the format requirement intact so
developers understand what value they need to generate.

In `@esignet-service/internal/config/db.go`:
- Around line 48-56: The code unconditionally appends `sslmode=disable` to any
PostgreSQL URL missing an explicit sslmode parameter, which silently disables
TLS and creates a security downgrade risk for production environments. Instead
of always appending `sslmode=disable` in the DSN construction block, add a check
to determine if the environment is development/local before applying this
default. For production environments, either require an explicit sslmode
parameter in the PostgreSQL URL or allow the driver to use its secure default.
This ensures that production configurations do not inadvertently have TLS
disabled.

In `@esignet-service/internal/engine/mosip/mosip_authn.go`:
- Around line 61-64: Add nil pointer checks for the metadata parameters at the
beginning of the Authenticate, GetAttributes, and SendOTP methods before
dereferencing them. In the Authenticate method around the
getApplicationAndClientID call, check if authnMetadata is nil and return an
appropriate error before accessing authnMetadata.RuntimeMetadata. Apply the same
defensive nil-check pattern to GetAttributes and SendOTP methods at their
respective locations (lines 205-208 and 242-245) to prevent panics from
dereferencing nil pointers.

In `@esignet-service/internal/engine/mosip/mosip_otp_executor.go`:
- Around line 101-108: The condition in the RuntimeData check for
ext_TransactionID only validates key existence but does not account for empty
values. Modify the condition that checks ctx.RuntimeData["ext_TransactionID"] to
treat both missing keys and empty string values as cases requiring
initialization. Update the logic to call GenerateTransactionID not just when the
key is absent, but also when the key exists but contains an empty value,
ensuring transaction correlation remains consistent across steps.

In `@esignet-service/README.md`:
- Around line 317-319: The Docker command publishes port 8088 but the
MOSIP_ESIGNET_HOST environment variable is incorrectly set to
http://127.0.0.1:8080, which creates a port mismatch. Update the
MOSIP_ESIGNET_HOST value to use port 8088 instead of 8080 to align with the
published container port in the docker run command, ensuring consistency in
OAuth flows and issuer base-url configuration.
🪄 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: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 65ff3379-7261-40fb-bfe8-8dc848ce892a

📥 Commits

Reviewing files that changed from the base of the PR and between 0821ca2 and a5e4b55.

📒 Files selected for processing (14)
  • esignet-service/.env.example
  • esignet-service/README.md
  • esignet-service/data/deployment.yaml
  • esignet-service/internal/clientmgmt/config.go
  • esignet-service/internal/clientmgmt/service.go
  • esignet-service/internal/config/db.go
  • esignet-service/internal/engine/authn_factory.go
  • esignet-service/internal/engine/config.go
  • esignet-service/internal/engine/consent.go
  • esignet-service/internal/engine/mock/mock_authn.go
  • esignet-service/internal/engine/mosip/config.go
  • esignet-service/internal/engine/mosip/mosip_authn.go
  • esignet-service/internal/engine/mosip/mosip_otp_executor.go
  • esignet-service/make.sh

Comment thread esignet-service/.env.example Outdated
Comment thread esignet-service/internal/config/db.go
Comment thread esignet-service/internal/engine/mosip/mosip_authn.go
Comment thread esignet-service/internal/engine/mosip/mosip_otp_executor.go
Comment thread esignet-service/README.md
Signed-off-by: anushasunkada <anushasunkada@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant