Skip to content

feat(oauth-providers): availbility to pass state into oauth middlewares#917

Closed
buiducnhat wants to merge 1 commit intohonojs:mainfrom
buiducnhat:availbility-pass-state-oauth-providers
Closed

feat(oauth-providers): availbility to pass state into oauth middlewares#917
buiducnhat wants to merge 1 commit intohonojs:mainfrom
buiducnhat:availbility-pass-state-oauth-providers

Conversation

@buiducnhat
Copy link

@buiducnhat buiducnhat commented Dec 30, 2024

Which middleware is the feature for?

@hono/oauth-providers

What is the feature you are proposing?

Currently, I see that @hono/oauth-providers supports discord, facebook, github, google, linkedin, x OAuth. I have just test for github and google and it works probably.

But I face a problem, I want to configure the URL that clients can be redirected after authorized successfully (redirect from Hono context, not from the providers, they are callbacks).

Then, I see that the googleAuth.ts has a options that can pass the state:

export function googleAuth(options: {
  scope: string[]
  login_hint?: string
  prompt?: 'none' | 'consent' | 'select_account'
  access_type?: 'online' | 'offline'
  client_id?: string
  client_secret?: string
  state?: string
  redirect_uri?: string
}): MiddlewareHandler {
  return async (c, next) => {
    const newState = options.state || getRandomState()

Then I can pass the clientRedirectUrl to the state, and extract it later.

But when dealing with Github (or other providers like the code I see), they don't have parameter state?

export function githubAuth(options: {
  client_id?: string
  client_secret?: string
  scope?: GitHubScope[]
  oauthApp?: boolean
  redirect_uri?: string
}): MiddlewareHandler {
  return async (c, next) => {
    const newState = getRandomState()

The above block is the githubAuth.ts for example

Are there any reasons that we cannot pass the state to the middlewares? From my side, I think it's inconsistent when we do like that. And of course, I love to have a parameter "state", it will help for my case.

@yusukebe
Copy link
Member

yusukebe commented Jan 6, 2025

@buiducnhat Thank you for the PR.

Hey @monoald, Can you handle this?

@buiducnhat
Copy link
Author

buiducnhat commented Feb 13, 2025

@buiducnhat Thank you for the PR.

Hey @monoald, Can you handle this?

@yusukebe Has my PR progress yet?

@yusukebe
Copy link
Member

@buiducnhat

Can you add tests?

@buiducnhat buiducnhat closed this by deleting the head repository Aug 17, 2025
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.

2 participants