[PR] fix(feishu-auth, dingtalk-auth): bind the authorization code to the session that started the login [apisix]

["shreemaan-abhishek (via GitHub)" <gi...@apache.org>]

shreemaan-abhishek opened a new pull request, #13806:
URL: https://github.com/apache/apisix/pull/13806

   ### Description
   
   `feishu-auth` and `dingtalk-auth` implement the OAuth 2.0 authorization code flow, but neither carried a `state` parameter across it. The plugin emitted the login redirect with no `state`, and on the way back it took the code straight out of the query string and exchanged it, with nothing tying that code to the browser that started the flow. A code obtained elsewhere was therefore accepted on any session.
   
   This PR closes that gap:
   
   - Before redirecting to `redirect_uri`, generate a random 16-byte `state`, store it in the session cookie, and append it to the redirect URL (`?state=` or `&state=` depending on whether the configured URL already has a query string).
   - When the code arrives in the query string, require the `state` to match the one bound to the session, and respond `401 {"message":"Invalid state"}` when it does not. The state is dropped from the session once used.
   - When the code arrives in the configured header (`X-Feishu-Code` / `X-DingTalk-Code`), no `state` is required. Those requests come from non-browser clients, which cannot be driven cross-site.
   
   This mirrors what `authz-casdoor` already does in this repository (`apisix/plugins/authz-casdoor.lua:187-196` generates and stores, `:131-144` validates).
   
   Also in this PR: `feishu-auth` called `sess:delete(...)` on the token refresh path. `lua-resty-session` 4.x has no `delete` method, so that path raised `attempt to call method 'delete' (a nil value)` and returned 500. Replaced with `sess:set(key, nil)`, which is the supported way to drop a key. This is exercised by the new tests.
   
   #### Behaviour change
   
   Deployments that use the query-string code path now need `state` to survive the round trip: `redirect_uri` receives it as a query parameter, and the login page is expected to pass it through to the IdP so that it comes back on the callback. This is the standard OAuth handling of `state`. Two consequences worth calling out:
   
   - A user who was mid-flow across an upgrade (redirected before, returning after) has no `state` in their session and gets a `401`. Retrying the flow succeeds.
   - Deployments that drive the plugin entirely through the code header are unaffected.
   
   There is no new configuration option: an opt-out would just be a switch for turning the check back off.
   
   No plugin schema fields were added or changed.
   
   #### Which issue(s) this PR fixes:
   
   Fixes #
   
   ### Checklist
   
   - [x] I have explained the need for this PR and the problem it solves
   - [x] I have explained the changes or the new features added to this PR
   - [x] I have added tests corresponding to this change
   - [x] I have updated the documentation to reflect this change
   - [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(feishu-auth, dingtalk-auth): bind the authorization code to the session that started the login [apisix]

["shreemaan-abhishek (via GitHub)" <gi...@apache.org>]

shreemaan-abhishek merged PR #13806:
URL: https://github.com/apache/apisix/pull/13806


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org