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

Google Sign-On with new libraries #5777

Merged
merged 3 commits into from
Oct 12, 2022
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 12 additions & 20 deletions templates/login/login_core.html.twig
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
{% set srOnly = (showLabels == true) ? "" : "sr-only" %}
Copy link
Member

Choose a reason for hiding this comment

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

above line looks like an unrelated change. If so, would add it back.

<!DOCTYPE html>
<html lang="en">
<head>
{% if displayGoogleSignin %}
<meta name="google-signin-client_id" content="{{ googleSigninClientID|attr }}">
{% endif %}
<script src="https://accounts.google.com/gsi/client" async defer></script>
<div id="g_id_onload"
data-client_id="{{ googleSigninClientID }}"
Saparbek-Nagashibekov marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

need to html escape this via attr:

data-client_id="{{ googleSigninClientID|attr }}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, how to do that ? could you give your suggested change? I will add it. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@bradymiller bradymiller Sep 24, 2022

Choose a reason for hiding this comment

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

i should of been more clear. you pipe it through the attr to html escape it there (see line below):

data-client_id="{{ googleSigninClientID|attr }}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What does it mean "html escape this via attr" how can I do that? Could you give me sample. thanks

Copy link
Member

@bradymiller bradymiller Sep 24, 2022

Choose a reason for hiding this comment

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

here is what the line should look like (see where the |attr is in below line :) ):

data-client_id="{{ googleSigninClientID|attr }}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Brady,
made changes. please check commit 1d831fc
Thanks

data-callback="onSignInSuccess" data-auto_prompt="false">
</div>
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this block of code be within the above if conditional in line 4?
(ie. only shows if displayGoogleSignin)


<title>{{ title|text }} {{ "Login"|xlt }}</title>
{% block head %}{% endblock %}
<script src="{{ webroot|attr }}/interface/product_registration/product_registration_service.js?v={{ assetVersion|attr_url }}"></script>
Expand Down Expand Up @@ -103,7 +108,8 @@
</div>
{% endif %}

<div class="form-group d-flex justify-content-end">
<div class="form-group d-flex " style="justify-content: space-between !important;" >
{% if displayGoogleSignin %} <div class="g_id_signin" data-type="standard" ></div> {% endif %}
<button id="login-button" class="btn btn-primary" type="submit" onclick="transmit_form(this)">{{ "Login"|xlt }}</button>
</div>

Expand Down Expand Up @@ -203,23 +209,22 @@

// Click-handler for signin button
function do_google_signin() {
google_signin = true;
google_signin = true;
}

// When Google sign-in successful, sign in to the app, but only
// if the button was clicked (otherwise we would automatically login)
function onSignInSuccess(googleUser) {
if (google_signin === true) {
const auth_response = googleUser.getAuthResponse();
const id_token = auth_response.id_token;

const id_token = googleUser.credential;
$('.login-failure').hide();
$('#used-google-signin').val(true);
$('#google-signin-token').val(id_token);
$('#google-signout').show();
$('#standard-auth-username, #standard-auth-password').hide();
var element = document.getElementById('login-button');
transmit_form(element);
}

}

function onSignInFailure(error) {
Expand Down Expand Up @@ -250,18 +255,5 @@
});
}

$.getScript('https://apis.google.com/js/platform.js', function (data, textStatus, jqxh) {
// When the auth2 library is loaded, log out so the user has to sign into google
gapi.load('auth2', function () {
gapi.auth2.init().then(function () {
signOut();
});
});

// Render the "Sign in with Google" button
renderButton();
}).fail(function (jqxhr, settings, exception) {
$('#google-signin-service-unreachable-alert').show();
});
</script>
{% endif %}