Fix: Resolve contradictory Gmail configs and add missing SMTP link #3170#6027
Fix: Resolve contradictory Gmail configs and add missing SMTP link #3170#6027SaviruDesilva wants to merge 1 commit intowso2:masterfrom
Conversation
|
|
WalkthroughDocumentation update to the Email OTP configuration guide that adds a prerequisite note about email server setup and simplifies the sample deployment configuration in Option 2 from a flat structure to a minimal array-style format, while adding an output event adapter email field. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@en/identity-server/5.10.0/docs/learn/configuring-email-otp.md`:
- Around line 404-418: The configuration uses inconsistent parameter formats:
replace the array-style parameter blocks
([[authentication.authenticator.email_otp.parameters]]) with the flat-style
single blocks ([authentication.authenticator.email_otp.parameters]) and convert
each name/value pair into direct key-value entries (e.g.,
EmailOTPAuthenticationEndpointURL, EmailOTPAuthenticationEndpointErrorPage,
EmailAddressRequestPage) under the
[authentication.authenticator.email_otp.parameters] section so the EmailOTP
authenticator configuration matches the flat-style used elsewhere (as in Option1
and passwordless-authentication-using-emailotp.md).
- Line 402: The note "Sample configuration when using Identity Server as Email
OTP Provider" is misplaced under the "Option2: Configure Gmail as the email OTP
provider" section; either move that configuration snippet into the "Option1:
Configure Identity Server as the email OTP provider" section (so the snippet
matches the note), or change the note text to reflect Gmail (e.g., "Sample
configuration when using Gmail as the Email OTP Provider") and update the
snippet to include Gmail-specific parameters; locate the note by the exact
string "Sample configuration when using Identity Server as Email OTP Provider"
and the two headings "Option1: Configure Identity Server as the email OTP
provider" and "Option2: Configure Gmail as the email OTP provider" when making
the change.
- Around line 404-422: The TOML example under
authentication.authenticator.email_otp has two problems: replace the incorrect
section name "[output_event_adapter.email]" with "[output_adapter.email]" and
add the missing Gmail authentication parameter entries under
[[authentication.authenticator.email_otp.parameters]] — specifically add
parameters named "GmailClientId", "GmailClientSecret", "GmailRefreshToken",
"GmailEmailEndpoint" (with the Gmail send URL) and "accessTokenRequiredAPIs"
(value "Gmail") so the Gmail OTP integration has credentials and endpoint; keep
the existing EmailOTPAuthenticationEndpointURL,
EmailOTPAuthenticationEndpointErrorPage, and EmailAddressRequestPage entries
intact.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5d1a1fd9-09dc-4f67-a512-8a1d6f27a95b
📒 Files selected for processing (1)
en/identity-server/5.10.0/docs/learn/configuring-email-otp.md
| @@ -401,22 +402,23 @@ SendGrid APIs. Follow the instructions in **one** of **Option1** or | |||
| !!! Note "Sample configuration when using Identity Server as Email OTP Provider" | |||
There was a problem hiding this comment.
Critical: Contradictory note placement.
The note states "Sample configuration when using Identity Server as Email OTP Provider", but this configuration snippet is located inside Option2: Configure Gmail as the email OTP provider (line 267). Option1 is for using Identity Server as the provider. This contradiction will confuse users about which configuration to use.
📝 Proposed fix
Either:
- Move this configuration to Option1 if it's meant for Identity Server as the OTP provider, or
- Update the note to reflect Gmail configuration:
-!!! Note "Sample configuration when using Identity Server as Email OTP Provider"
+!!! Note "Sample configuration when using Gmail as Email OTP Provider"However, note that the configuration is also missing required Gmail-specific parameters (see separate comment).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| !!! Note "Sample configuration when using Identity Server as Email OTP Provider" | |
| !!! Note "Sample configuration when using Gmail as Email OTP Provider" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@en/identity-server/5.10.0/docs/learn/configuring-email-otp.md` at line 402,
The note "Sample configuration when using Identity Server as Email OTP Provider"
is misplaced under the "Option2: Configure Gmail as the email OTP provider"
section; either move that configuration snippet into the "Option1: Configure
Identity Server as the email OTP provider" section (so the snippet matches the
note), or change the note text to reflect Gmail (e.g., "Sample configuration
when using Gmail as the Email OTP Provider") and update the snippet to include
Gmail-specific parameters; locate the note by the exact string "Sample
configuration when using Identity Server as Email OTP Provider" and the two
headings "Option1: Configure Identity Server as the email OTP provider" and
"Option2: Configure Gmail as the email OTP provider" when making the change.
| [authentication.authenticator.email_otp] | ||
| name = "EmailOTP" | ||
| enable= true | ||
| [authentication.authenticator.email_otp.parameters] | ||
| EMAILOTPAuthenticationEndpointURL = "https://localhost:9443/emailotpauthenticationendpoint/emailotp.jsp" | ||
| EmailOTPAuthenticationEndpointErrorPage = "https://localhost:9443/emailotpauthenticationendpoint/emailotpError.jsp" | ||
| EmailAddressRequestPage = "https://localhost:9443/emailotpauthenticationendpoint/emailAddress.jsp" | ||
| usecase = "association" | ||
| secondaryUserstore = "primary" | ||
| EMAILOTPMandatory = false | ||
| sendOTPToFederatedEmailAttribute = false | ||
| federatedEmailAttributeKey = "email" | ||
| EmailOTPEnableByUserClaim = true | ||
| CaptureAndUpdateEmailAddress = true | ||
| showEmailAddressInUI = true | ||
| useEventHandlerBasedEmailSender = true | ||
| emailAddressRegex = '(?<=.{1}).(?=.*@)' | ||
| name = "EmailOTP" | ||
| enable = true | ||
|
|
||
| [[authentication.authenticator.email_otp.parameters]] | ||
| name = "EmailOTPAuthenticationEndpointURL" | ||
| value = "https://localhost:9443/emailotpauthenticationendpoint/emailotp.jsp" | ||
|
|
||
| [[authentication.authenticator.email_otp.parameters]] | ||
| name = "EmailOTPAuthenticationEndpointErrorPage" | ||
| value = "https://localhost:9443/emailotpauthenticationendpoint/emailotpError.jsp" | ||
|
|
||
| [[authentication.authenticator.email_otp.parameters]] | ||
| name = "EmailAddressRequestPage" | ||
| value = "https://localhost:9443/emailotpauthenticationendpoint/emailAddress.jsp" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Major: Inconsistent configuration format between Option1 and Option2.
Option1 (lines 51-70) uses the flat-style format with [authentication.authenticator.email_otp.parameters] and direct key-value pairs, while Option2 now uses the array-style format with [[authentication.authenticator.email_otp.parameters]] and name/value pairs.
This inconsistency creates confusion about which format to use and may suggest different configuration requirements when there shouldn't be any. Both options should use the same configuration format for consistency unless there's a technical reason for the difference.
Consider standardizing on the flat-style format used in Option1, as shown in the previous comment's recommended fix. This matches the format used elsewhere in the WSO2 Identity Server 5.10.0 documentation (see context snippet from passwordless-authentication-using-emailotp.md).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@en/identity-server/5.10.0/docs/learn/configuring-email-otp.md` around lines
404 - 418, The configuration uses inconsistent parameter formats: replace the
array-style parameter blocks
([[authentication.authenticator.email_otp.parameters]]) with the flat-style
single blocks ([authentication.authenticator.email_otp.parameters]) and convert
each name/value pair into direct key-value entries (e.g.,
EmailOTPAuthenticationEndpointURL, EmailOTPAuthenticationEndpointErrorPage,
EmailAddressRequestPage) under the
[authentication.authenticator.email_otp.parameters] section so the EmailOTP
authenticator configuration matches the flat-style used elsewhere (as in Option1
and passwordless-authentication-using-emailotp.md).
| [authentication.authenticator.email_otp] | ||
| name = "EmailOTP" | ||
| enable= true | ||
| [authentication.authenticator.email_otp.parameters] | ||
| EMAILOTPAuthenticationEndpointURL = "https://localhost:9443/emailotpauthenticationendpoint/emailotp.jsp" | ||
| EmailOTPAuthenticationEndpointErrorPage = "https://localhost:9443/emailotpauthenticationendpoint/emailotpError.jsp" | ||
| EmailAddressRequestPage = "https://localhost:9443/emailotpauthenticationendpoint/emailAddress.jsp" | ||
| usecase = "association" | ||
| secondaryUserstore = "primary" | ||
| EMAILOTPMandatory = false | ||
| sendOTPToFederatedEmailAttribute = false | ||
| federatedEmailAttributeKey = "email" | ||
| EmailOTPEnableByUserClaim = true | ||
| CaptureAndUpdateEmailAddress = true | ||
| showEmailAddressInUI = true | ||
| useEventHandlerBasedEmailSender = true | ||
| emailAddressRegex = '(?<=.{1}).(?=.*@)' | ||
| name = "EmailOTP" | ||
| enable = true | ||
|
|
||
| [[authentication.authenticator.email_otp.parameters]] | ||
| name = "EmailOTPAuthenticationEndpointURL" | ||
| value = "https://localhost:9443/emailotpauthenticationendpoint/emailotp.jsp" | ||
|
|
||
| [[authentication.authenticator.email_otp.parameters]] | ||
| name = "EmailOTPAuthenticationEndpointErrorPage" | ||
| value = "https://localhost:9443/emailotpauthenticationendpoint/emailotpError.jsp" | ||
|
|
||
| [[authentication.authenticator.email_otp.parameters]] | ||
| name = "EmailAddressRequestPage" | ||
| value = "https://localhost:9443/emailotpauthenticationendpoint/emailAddress.jsp" | ||
|
|
||
| [output_event_adapter.email] | ||
| default_from_address = "your_email@gmail.com" | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for output adapter email configuration patterns in documentation and config files
echo "=== Searching for output adapter email config patterns ==="
rg -n "output.*adapter.*email" --type toml --type md -C2
echo ""
echo "=== Searching for specific section names ==="
rg -n "\[output_event_adapter\.email\]|\[output_adapter\.email\]" --type toml --type md -C2Repository: wso2/docs-is
Length of output: 50370
🏁 Script executed:
cat -n en/identity-server/5.10.0/docs/learn/configuring-email-otp.md | sed -n '374-475p'Repository: wso2/docs-is
Length of output: 107
🏁 Script executed:
cat -n en/identity-server/5.10.0/docs/learn/configuring-email-otp.md | sed -n '374,475p'Repository: wso2/docs-is
Length of output: 6080
🏁 Script executed:
cat -n en/identity-server/5.10.0/docs/learn/configuring-email-otp.md | sed -n '1,403p' | tail -50Repository: wso2/docs-is
Length of output: 2858
🏁 Script executed:
cat -n en/identity-server/5.10.0/docs/learn/configuring-email-otp.md | sed -n '270,330p'Repository: wso2/docs-is
Length of output: 3426
🏁 Script executed:
cat -n en/identity-server/5.10.0/docs/learn/configuring-email-otp.md | sed -n '475,530p'Repository: wso2/docs-is
Length of output: 2922
🏁 Script executed:
# Check the format used for email_otp.parameters in the file
cat -n en/identity-server/5.10.0/docs/learn/configuring-email-otp.md | sed -n '404,425p'Repository: wso2/docs-is
Length of output: 1008
🏁 Script executed:
# Search for any other configuration examples in the file that include Gmail parameters
rg -n "GmailClientId|GmailClientSecret|GmailRefreshToken|GmailEmailEndpoint" en/identity-server/5.10.0/docs/learn/configuring-email-otp.md -A 2 -B 2Repository: wso2/docs-is
Length of output: 1633
🏁 Script executed:
# Check what comes before line 402 to understand which Option this is for
cat -n en/identity-server/5.10.0/docs/learn/configuring-email-otp.md | sed -n '250,275p'Repository: wso2/docs-is
Length of output: 1259
Fix incorrect section name and add missing Gmail authentication parameters to the configuration example.
The configuration at lines 404-422 has two critical issues:
-
Incorrect section name at line 420: The section is named
[output_event_adapter.email], but the correct name across the entire codebase is[output_adapter.email]. -
Missing Gmail authentication parameters: The configuration snippet only shows endpoint parameters but omits the Gmail-specific credentials documented in the parameter table below (lines 448-495). When using Gmail as the OTP provider, the configuration must include:
GmailClientIdGmailClientSecretGmailRefreshTokenGmailEmailEndpointaccessTokenRequiredAPIs
Without these parameters, the Gmail integration will fail. The configuration example should be corrected to include all required Gmail authentication details.
Configuration format
[authentication.authenticator.email_otp]
name = "EmailOTP"
enable = true
[[authentication.authenticator.email_otp.parameters]]
name = "EmailOTPAuthenticationEndpointURL"
value = "https://localhost:9443/emailotpauthenticationendpoint/emailotp.jsp"
[[authentication.authenticator.email_otp.parameters]]
name = "EmailOTPAuthenticationEndpointErrorPage"
value = "https://localhost:9443/emailotpauthenticationendpoint/emailotpError.jsp"
[[authentication.authenticator.email_otp.parameters]]
name = "EmailAddressRequestPage"
value = "https://localhost:9443/emailotpauthenticationendpoint/emailAddress.jsp"
[[authentication.authenticator.email_otp.parameters]]
name = "GmailClientId"
value = "<your_client_id>"
[[authentication.authenticator.email_otp.parameters]]
name = "GmailClientSecret"
value = "<your_client_secret>"
[[authentication.authenticator.email_otp.parameters]]
name = "GmailRefreshToken"
value = "<your_refresh_token>"
[[authentication.authenticator.email_otp.parameters]]
name = "GmailEmailEndpoint"
value = "https://www.googleapis.com/gmail/v1/users/<your_email@gmail.com>/messages/send"
[[authentication.authenticator.email_otp.parameters]]
name = "accessTokenRequiredAPIs"
value = "Gmail"
[output_adapter.email]
default_from_address = "your_email@gmail.com"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@en/identity-server/5.10.0/docs/learn/configuring-email-otp.md` around lines
404 - 422, The TOML example under authentication.authenticator.email_otp has two
problems: replace the incorrect section name "[output_event_adapter.email]" with
"[output_adapter.email]" and add the missing Gmail authentication parameter
entries under [[authentication.authenticator.email_otp.parameters]] —
specifically add parameters named "GmailClientId", "GmailClientSecret",
"GmailRefreshToken", "GmailEmailEndpoint" (with the Gmail send URL) and
"accessTokenRequiredAPIs" (value "Gmail") so the Gmail OTP integration has
credentials and endpoint; keep the existing EmailOTPAuthenticationEndpointURL,
EmailOTPAuthenticationEndpointErrorPage, and EmailAddressRequestPage entries
intact.
2 participants
Summary:
This PR addresses documentation issue #3170 in the Email OTP guide.
Changes:
Affected Version: 5.10.0
Note: I am a 3rd-year CS student at the University of Ruhuna contributing to WSO2 for the first time.
Summary by CodeRabbit