Repository navigation
fix: always verify the id_token for email regardless of token scope - #6
Merged
lipowen merged 1 commit intoSep 8, 2026
Conversation
…TEACH-22605) LINE's token response never lists `email` in its `scope` field, even when the user granted the email permission (LINE Login v2.1 reference). The guard added in 4fc4906 therefore skipped the verify call on every real login and made the email always nil; it is removed so any issued id_token is verified. The shared spec fixture now uses LINE's real scope string (`profile openid`) so the happy-path email example exercises the production shape, the context that encoded the wrong scope model is dropped, and a context covers the not-granted case: verify answers 200 without an `email` claim and `info[:email]` is nil.
lipowen
force-pushed
the
feature/teach-22605-omniauth-line-remove-the-scope-guard-that-drops-line-email
branch
from
September 8, 2026 02:37
11e454a to
7af0722
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes LINE email retrieval by ensuring the strategy always verifies the id_token (when present) to decode the email claim, instead of gating the verify call on the token response’s scope field (which LINE doesn’t include email in).
Changes:
- Removes the
scope-based early return fromOmniAuth::Strategies::Line#email, keeping only theid_tokenpresence guard. - Updates specs to reflect LINE’s real token
scope(profile openid) and adds coverage for the “email permission not granted” verify response shape (200 without anemailclaim).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/omniauth/strategies/line.rb | Drops scope guard so email verification happens whenever an id_token exists. |
| spec/omniauth/strategies/line_spec.rb | Updates fixtures to match real scopes and adds a no-email-claim verify response scenario. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+149
to
155
| context 'when the user did not grant the email permission' do | ||
| before do | ||
| stub_request(:post, 'https://api.line.me/oauth2/v2.1/verify') | ||
| .to_return(:status => 200, | ||
| :body => { 'iss' => 'https://access.line.me', 'sub' => 'U1234567890' }.to_json, | ||
| :headers => { 'Content-Type' => 'application/json' }) | ||
| end |
linyiru
approved these changes
Sep 8, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Removes the
scopeguard that #3 (4fc4906) added toOmniAuth::Strategies::Line#email:LINE's token response never lists
emailinscope, even when the user granted the email permission. From the LINE Login v2.1 reference: "Even if theemailpermission is granted, thescopeproperty doesn't includeemail." The email claim exists only inside the ID token and is read viaPOST /oauth2/v2.1/verify. With the guard in place every real login skipped the verify call, soinfo[:email]was alwaysnilonmaster(ffeb2a4). The gem's suite passed only because the fixture stubbedscope: 'profile openid email', a value LINE does not return; the live E2E noted in #3 predates that commit.Impact on kaikhq/manekineko is limited: its
User.from_omniauthdecodes the email fromextra[:id_token]itself for LINE and never readsinfo[:email], andextra[:id_token]is unaffected by the guard. Production also still pins8efe18d. kaikhq/manekineko#16165 is therefore not broken byffeb2a4, but should still pick up this fix soinfo[:email]is correct for every consumer.Changes
lib/omniauth/strategies/line.rb: drop the scope check; keepreturn nil unless id_token. The resulting file is byte-identical to the strategy immediately before4fc4906.spec/omniauth/strategies/line_spec.rb: the shared token fixture uses LINE's real scope string (profile openid) so the happy-path email example fails against the guarded code; the context encoding the wrong scope model is removed; a new context covers the not-granted case (verify answers 200 without anemailclaim →info[:email]isnil).Verification
bundle exec rake: 19 examples, 0 failures, 100% line coverage.master'sline.rbunder the new spec turns 2 examples red (email nil; the timeout example no longer reaches verify), so the spec does guard the regression.Follow-ups
emailmethod carried the same guard and the same fix applies.ETIMEDOUT— pre-existing (also on8efe18d), and Make configured options reach LINE and harden the outbound calls #4 adds timeouts plus degradation tonil.Linear: TEACH-22605