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

Fixed wrong implementation of Basic auth #8779

Merged
merged 2 commits into from
Mar 1, 2023

Conversation

GGGuenni
Copy link
Collaborator

@GGGuenni GGGuenni commented Mar 1, 2023

  • added min length (6) for passwords

Basic auth is supposed to base64 encode the whole userId:password string instead of encoding userId and password separately

@touhidurrr and @oynqr since you've already started implementing auth

+ added min length for passwords
@oynqr
Copy link
Contributor

oynqr commented Mar 1, 2023

This still compresses the string before base64 encoding it, no? It's not supposed to be compressed.

+ added function to get auth header from settings
@yairm210 yairm210 merged commit 50da1ee into yairm210:master Mar 1, 2023
@touhidurrr
Copy link
Contributor

@GGGuenni is auth version still 1 or 2?

@GGGuenni
Copy link
Collaborator Author

GGGuenni commented Mar 2, 2023

Still 1

@touhidurrr
Copy link
Contributor

oh, also why is argon2 verify not working? am I getting something wrong here? isn't the put request body the new password?

import * as argon2 from 'argon2';
import type { FastifyRequest } from 'fastify/types/request';

const idRegex = /^[\da-f]{8}-([\da-f]{4}-){3}[\da-f]{12}$/;

const putAuth = async (req: FastifyRequest<{ Body: string }>) => {
  const { userId, body, server } = req;
  const hash = await argon2.hash(body);

  const isClientId = idRegex.test(userId!);
  await Promise.all([
    server.cache.auth.set(userId!, hash),
    server.db.Auth.updateOne(
      { _id: userId },
      { $set: { hash, type: isClientId ? 'client' : 'discord', timestamp: Date.now() } },
      { upsert: true }
    ),
  ]);
  return '200 OK!';
};

export default putAuth;

@GGGuenni
Copy link
Collaborator Author

GGGuenni commented Mar 2, 2023

It is but I forgot to remove the compression from the password sorry!
Will make a PR

@touhidurrr
Copy link
Contributor

ok

@GGGuenni GGGuenni deleted the auth-changes branch March 2, 2023 13:38
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.

4 participants