Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: add types #140

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Conversation

Barbapapazes
Copy link
Contributor

@Barbapapazes Barbapapazes commented Aug 17, 2024

Hello 👋,

This is a proposal to add more types and to standardize the OAuthUser and OAuthToken in order to simplify the usage. This is inspire by the adonis social authentication package, https://docs.adonisjs.com/guides/authentication/social-authentication#user-properties, and laravel socialite package, https://github.com/laravel/socialite/blob/5.x/src/AbstractUser.php#L8. I also read the RFC6749

For this first draft, I update the GitHub and Discord providers.

Happy to have some feedback about it!

  • auth0
  • battledotnet (unable to find documentation about the /oauth/userinfo)
  • cognito
  • discord
  • facebook
  • github
  • google
  • keycloak
  • linkedin
  • microsoft
  • paypal
  • spotify
  • stream
  • twitch
  • x
  • xsuaa
  • yandex

How to help?

  1. Finding documentation about the userinfo response of each unchecked provider
  2. Providing the <Provider>User type

Example

/**
 * GitHub User
 *
 * @see https://docs.github.com/en/rest/users/users?apiVersion=2022-11-28#get-the-authenticated-user
 */
type GitHubUser = {
  login: string
  id: number
  node_id: string
  avatar_url: string
  name: string
  email: string

  [key: string]: unknown
}
  1. Trying refactored providers to detect potential issues
  2. Refactoring a provider and making a PR against mine

@atinux
Copy link
Owner

atinux commented Aug 19, 2024

I really like the idea @Barbapapazes

What I am afraid is potentially the maintenance might be harder but I think it's worth it, do you feel like updating all the providers?

Also, this PR is a breaking change as we don't return the same data anymore.

@atinux atinux added the enhancement New feature or request label Aug 19, 2024
@Barbapapazes Barbapapazes changed the title feat: add types feat!: add types Aug 26, 2024
@sandros94
Copy link
Contributor

One that I know of is Paypal doc's link:

/**
 * Paypal User
 *
 * @see https://developer.paypal.com/docs/api/identity/v1/#userinfo_get
 */
type PaypalUser = {
  user_id: string
  sub: string
  name: string
  given_name: string
  family_name: string
  middle_name: string
  picture: string
  email: string
  email_verified: boolean
  gender: string
  birthdate: string
  zoneinfo: string
  locale: string
  phone_number: string
  address: {
    street_address: string
    locality: string
    region: string
    postal_code: string
    country: string
  }
  verified_account: boolean
  account_type: 'PERSONAL' | 'BUSINESS' | 'PREMIER'
  age_range: string
}

As I get some free time I'll hope to help looking for missing ones

@IsraelOrtuno
Copy link
Contributor

IsraelOrtuno commented Sep 20, 2024

I like the initiative as I am a Laravel Socialite user and this works fine for most cases. Would be nice to also include the raw user response object too, what do you think?

    return onSuccess(event, {
      user: normalizeAuth0User(user),
      tokens: normalizeAuth0Tokens(tokens as OAuthAccessTokenSuccess),
      _data: rawUserInfoResponse
    })

This is just an example but would be nice to access some provider specific data.

Edit: My bad I just was it's nested in the user object, good enough!:
https://github.com/atinux/nuxt-auth-utils/pull/140/files#diff-73676617675abc1f04e949cf6db049e90c79d9016afe08182042a09dd113b32aR171

@Velka-DEV
Copy link
Contributor

Velka-DEV commented Nov 6, 2024

Im not sure what is the actual state of this PR, but i think it could be more reliable / easier to allow for type casting when we retrieve the session with the useUserSession composable (e.g useUserSession<MyAppUser>()).

@sandros94
Copy link
Contributor

Im not sure what is the actual state of this PR, but i think it could be more reliable / easier to allow for type casting when we retrieve the session with the useUserSession composable (e.g useUserSession()).

No need to type cast atm, we already have auth.d.ts for it:

// auth.d.ts
declare module '#auth-utils' {
  interface User {
    // Add your own fields
  }

  interface UserSession {
    // Add your own fields
  }

  interface SecureSessionData {
    // Add your own fields
  }
}

export {}

@Velka-DEV
Copy link
Contributor

Im not sure what is the actual state of this PR, but i think it could be more reliable / easier to allow for type casting when we retrieve the session with the useUserSession composable (e.g useUserSession()).

No need to type cast atm, we already have auth.d.ts for it:

// auth.d.ts
declare module '#auth-utils' {
  interface User {
    // Add your own fields
  }

  interface UserSession {
    // Add your own fields
  }

  interface SecureSessionData {
    // Add your own fields
  }
}

export {}

Yeah, I know, I’m just not convinced by the utility of adding hard-typed user types, as most claims are request-based on the scopes.

@sandros94
Copy link
Contributor

Yeah, I know

Ah, my bad 🙌

I’m just not convinced by the utility of adding hard-typed user types, as most claims are request-based on the scopes.

Personally I would say that having it act more like a ground truth between the different oauth providers and your own application logic.
Otherwise each time I need to add a new oauth provider I also need to remember and include additional logics into my Vue bundle just to properly display the user's profile name and image. Just to give an example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants