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

Add osu! client verification support #2862

Closed
Fabricio20 opened this issue Mar 30, 2018 · 6 comments · Fixed by #5316
Closed

Add osu! client verification support #2862

Fabricio20 opened this issue Mar 30, 2018 · 6 comments · Fixed by #5316

Comments

@Fabricio20
Copy link

Fabricio20 commented Mar 30, 2018

(Sorry if this is known/a duplicate - I tried search)

When logging in (first time) with the old osu! client we need to authorize our accounts, that means we are redirected to the old website which will be getting removed soon. I noticed osu!lazer doesn't require authorization however, so this may not be an issue (unless that feature is not yet implemented there).

@peppy
Copy link
Sponsor Member

peppy commented Apr 2, 2018

This will be updated in the near future.

@peppy peppy added this to the Candidate Issues milestone Apr 2, 2018
@peppy peppy changed the title Authorization / Authentication Page Client 2FA Authorization page is not implemented Apr 2, 2018
@xaoseric
Copy link

I'm working on the code for 2fa support and Yubikey support if there is a template that can be used for that.

@notbakaneko
Copy link
Collaborator

For the initial implementation, we'd want to give the option of a generated one-time passcode (like the one generated by Google authenticator or Authy) as a substitute for the verification codes that are currently sent out via email. Basically, if a user has this option enabled, it would replace the email-based verification codes, so the existing verification dialog can be used for input.

Other details like how account recovery would work in the case of forgotten passwords, etc; when 2FA is enabled are still undecided. We're not considering explicit support for U2F yet.

@LiquidPL
Copy link
Contributor

Is anyone working on this one currently? Would like to start working at least on the technical side of implementation.

@peppy
Copy link
Sponsor Member

peppy commented Oct 23, 2019

I was about to bring this up today. This does need priority as it is seemingly one of the two remaining systems left on the old site (the other one being arguable unnecessary, forum PMs).

Basic requirements for this:

  • Can use the existing 2FA verification page or a new one (whichever is easier to implement)
  • The client will send the user to the page, with an extra argument (POST or GET)
  • That argument contains client identifiers that will allow marking the user's game session as verified.
  • This should run even if the user is already in a verified web-session state.

Here's the old implementation for reference.

function updateClientHash()
{
	global $conn, $user;

	if ($_SESSION['clienthash'])
	{
		$clientHash = explode(':', sqlstr($_SESSION['clienthash']));
		
		$conn->exec("INSERT IGNORE INTO osu_user_security (user_id, osu_md5, mac_md5, unique_md5, disk_md5)	VALUES ({$user->data[user_id]} , UNHEX('{$clientHash[0]}'), UNHEX('{$clientHash[2]}'), UNHEX('{$clientHash[3]}'), UNHEX('{$clientHash[4]}'))");

		$conn->exec("UPDATE osu_user_security SET verified = 1 WHERE user_id = {$user->data[user_id]} AND unique_md5 = UNHEX('{$clientHash[3]}') AND osu_md5 = UNHEX('$clientHash[0]') AND verified = 0");
		$_SESSION['clienthash'] = null;
	}
}

I'd hope there are tests to go along with this, if possible.

Let me know if you have any further questions.

@cl8n
Copy link
Member

cl8n commented Oct 23, 2019

the name of this issue made me think it was about mobile app 2fa (seems like notbakaneko misunderstood it as that too), can we open a different issue for that and rename this one

@peppy peppy changed the title Client 2FA Authorization page is not implemented Add osu! client verification support Oct 23, 2019
@nanaya nanaya mentioned this issue Nov 21, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants