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

fix: allow handle format with tld shorter than 3 characters #3619

Conversation

Johannes-Andersen
Copy link

@Johannes-Andersen Johannes-Andersen commented Mar 8, 2025

When trying to sign up in the oAuth flow with a tld or domain shorter than 3 char, you end up getting blocked by zod validation against the post body.

Example of failing ones:
"hello.norwegian.no"
"hello.vg.com"

One could in theory move the logic from ensureHandleServiceConstraints https://github.com/Johannes-Andersen/atproto/blob/main/packages/pds/src/handle/index.ts#L26 to align the logic. But not familiar enough with the codebase to know how that would be done.

Logic can also be tested on https://zod-playground.vercel.app/

Ref implementation done by @matthieusieben in #2945

--
Error:
Screenshot 2025-03-08 at 23 57 20
Screenshot 2025-03-08 at 23 57 08

Stack from logs:

{
  "name": "pds:oauth",
  "err": {
    "type": "ZodError",
    "message": [
      {
        "validation": "regex",
        "code": "invalid_string",
        "message": "Invalid",
        "path": [
          "body",
          "handle"
        ]
      }
    ],
    "stack": "ZodError: [\n  {\n    \"validation\": \"regex\",\n    \"code\": \"invalid_string\",\n    \"message\": \"Invalid\",\n    \"path\": [\n      \"body\",\n      \"handle\"\n    ]\n  }\n]\n    at get error (/app/node_modules/.pnpm/zod@3.24.1/node_modules/zod/lib/types.js:55:31)\n    at ZodObject.parseAsync (/app/node_modules/.pnpm/zod@3.24.1/node_modules/zod/lib/types.js:196:22)\n    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)\n    at async <anonymous> (/app/node_modules/.pnpm/@atproto+oauth-provider@0.5.1/node_modules/@atproto/oauth-provider/src/oauth-provider.ts:1256:23)\n    at async <anonymous> (/app/node_modules/.pnpm/@atproto+oauth-provider@0.5.1/node_modules/@atproto/oauth-provider/src/oauth-provider.ts:1164:26)",
    "aggregateErrors": [
      {
        "type": "Object",
        "message": "Invalid",
        "stack": "",
        "validation": "regex",
        "code": "invalid_string",
        "path": [
          "body",
          "handle"
        ]
      }
    ],
    "issues": [
      {
        "validation": "regex",
        "code": "invalid_string",
        "message": "Invalid",
        "path": [
          "body",
          "handle"
        ]
      }
    ],
    "name": "ZodError"
  },
  "req": {
    "_parsedUrl": {
      "_raw": "/oauth/authorize/verify-handle-availability",
      "auth": null,
      "hash": null,
      "host": null,
      "hostname": null,
      "href": "/oauth/authorize/verify-handle-availability",
      "path": "/oauth/authorize/verify-handle-availability",
      "pathname": "/oauth/authorize/verify-handle-availability",
      "port": null,
      "protocol": null,
      "query": null,
      "search": null,
      "slashes": null
    },
    "method": "POST",
    "originalUrl": "/oauth/authorize/verify-handle-availability",
    "params": {},
    "query": {},
    "upgrade": false,
    "url": "/oauth/authorize/verify-handle-availability"
  },
  "msg": "OAuth request error"
}

message: 'Handle must be at least 3 characters long',
})
.refine((value) => value.split('.')[0].length <= 18, {
message: 'Handle must be at most 18 characters long',
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matthieusieben
Copy link
Contributor

Thank you for your contribution. This looks almost right 👍

@bnewbold
Copy link
Collaborator

bnewbold commented Mar 9, 2025

would it be clearer to validate the overall handle syntax using the @atproto/syntax library, and then check the additional "name" constraints after that? Instead of having checks for digits, dashes, etc duplicated here.

@Johannes-Andersen
Copy link
Author

Johannes-Andersen commented Mar 9, 2025

would it be clearer to validate the overall handle syntax using the @atproto/syntax library, and then check the additional "name" constraints after that? Instead of having checks for digits, dashes, etc duplicated here.

I agree!
I’ll have a go at it later in the week to see if I can get something up for review if someone doesn’t beat me to it.

Just need to figure out how to properly get it all running locally and wrap my head around the codebase and packages 😄

@matthieusieben
Copy link
Contributor

#3622

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.

3 participants