Skip to content

Handle broken response from userinfo_endpoint#3427

Merged
sigurdm merged 2 commits into
dart-lang:masterfrom
sigurdm:handle_broken_user_info
May 20, 2022
Merged

Handle broken response from userinfo_endpoint#3427
sigurdm merged 2 commits into
dart-lang:masterfrom
sigurdm:handle_broken_user_info

Conversation

@sigurdm

@sigurdm sigurdm commented May 16, 2022

Copy link
Copy Markdown
Contributor

Fix of: #3424

Comment thread lib/src/command/login.dart Outdated
log.message('You are now logged in as $userInfo');
if (userInfo == null) {
log.warning('Could not retrieve your user-details.\n'
'You might have to run `pub logout` to delete your credentials and try again.');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

extra space between credentials and and.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also say dart pub or use the variable we have for the top-level name

@sigurdm sigurdm merged commit 0ad17e8 into dart-lang:master May 20, 2022
@bsutton

bsutton commented May 22, 2022

Copy link
Copy Markdown
Contributor

I don't believe this patch is working.
I've applied the patch to a local copy of the code and I"m getting the following output:

Pub needs your authorization to upload packages on your behalf.
In a web browser, go to https://accounts.google.com/o/oauth2/auth?access_type=offline&approval_prompt=force&response_type=code&client_id=818368855108-8grd2eg9tj9f38os6f1urbcvsq399u8n.apps.googleusercontent.com&redirect_uri=http%3A%2F%2Flocalhost%3A42157&code_challenge=cmvaULtetJ_xhL-Wz8FSFLJxo8DY5ZdgmjN-yDfGpkQ&code_challenge_method=S256&scope=openid+https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fuserinfo.email
Then click "Allow access".

Waiting for your authorization...
Authorization received, processing...
Successfully authorized.
You are now logged in as null
Could not retrieve your user-details.
You might have to run `pub logout` to delete your credentials  and try again.

The symptom is that the 'name' field is not being returned by the google auth mechanism.
I've tried pub logout and then pub login with the same results.
Is there some chance that google no longer return the 'name' field in the user info response?

Reading the google documentation:
https://developers.google.com/identity/protocols/oauth2/openid-connect#server-flow

name - The user's full name, in a displayable form. Might be provided when: The request scope included the string "profile"
The ID token is returned from a token refresh. When name claims are present, you can use them to update your app's user records. 

Note that this claim is never guaranteed to be present.

Note the last sentence.

My suggestion is to change _UserInfo to:

class _UserInfo {
  final String? name;
  final String email;
  _UserInfo(this.name, this.email);
  @override
  String toString() => ['<$email>', name ?? ''].join(' ');
}

The name field doesn't appear to be used anywhere in the code except in the above toString call.

Here is the full patch (sorry I'm unable to raise a PR at this time.

// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:async';
import 'dart:convert';

import '../command.dart';
import '../http.dart';
import '../log.dart' as log;
import '../oauth2.dart' as oauth2;

/// Handles the `login` pub command.
class LoginCommand extends PubCommand {
  @override
  String get name => 'login';
  @override
  String get description => 'Log into pub.dev.';
  @override
  String get invocation => 'pub login';

  LoginCommand();

  @override
  Future<void> runProtected() async {
    final credentials = oauth2.loadCredentials(cache);
    if (credentials == null) {
      final userInfo = await _retrieveUserInfo();
      if (userInfo == null) {
        log.warning('Could not retrieve your user-details.\n'
            'You might have to run `pub logout` to delete your credentials  and try again.');
      } else {
        log.message('You are now logged in as $userInfo');
      }
    } else {
      final userInfo = await _retrieveUserInfo();
      if (userInfo == null) {
        log.warning('Your credentials seems broken.\n'
            'Run `pub logout` to delete your credentials and try again.');
      } else {
        log.warning('You are already logged in as $userInfo\n'
            'Run `pub logout` to log out and try again.');
      }
    }
  }

  Future<_UserInfo?> _retrieveUserInfo() async {
    return await oauth2.withClient(cache, (client) async {
      final discovery = await httpClient.get(Uri.https(
          'accounts.google.com', '/.well-known/openid-configuration'));
      final userInfoEndpoint = json.decode(discovery.body)['userinfo_endpoint'];
      final userInfoRequest = await client.get(Uri.parse(userInfoEndpoint));
      if (userInfoRequest.statusCode != 200) return null;
      try {
        final userInfo = json.decode(userInfoRequest.body);
        final name = userInfo['name'];
        final email = userInfo['email'];
        if (email is String) {
          return _UserInfo(name, email);
        } else {
          log.fine(
              'Bad response from $userInfoEndpoint: ${userInfoRequest.body}');
          return null;
        }
      } on FormatException catch (e) {
        log.fine(
            'Bad response from $userInfoEndpoint ($e): ${userInfoRequest.body}');
        return null;
      }
    });
  }
}

class _UserInfo {
  final String? name;
  final String email;
  _UserInfo(this.name, this.email);
  @override
  String toString() => ['<$email>', name ?? ''].join(' ');
}

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