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

The user is set on the login screen if transmitted from the client #67

Merged
merged 1 commit into from
Aug 1, 2017

Conversation

DeepDiver1975
Copy link
Member

fixes #66

@DeepDiver1975 DeepDiver1975 added this to the development milestone Jul 31, 2017
@DeepDiver1975 DeepDiver1975 self-assigned this Jul 31, 2017
@codecov
Copy link

codecov bot commented Jul 31, 2017

Codecov Report

Merging #67 into master will decrease coverage by 0.39%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master      #67     +/-   ##
===========================================
- Coverage     84.33%   83.93%   -0.4%     
- Complexity      179      180      +1     
===========================================
  Files            20       20             
  Lines           632      635      +3     
===========================================
  Hits            533      533             
- Misses           99      102      +3
Impacted Files Coverage Δ Complexity Δ
lib/AppInfo/Application.php 22.44% <0%> (-1.47%) 12 <0> (+1)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5846a0f...75e88e6. Read the comment docs.

Copy link
Contributor

@SamuAlfageme SamuAlfageme left a comment

Choose a reason for hiding this comment

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

I'm still getting a redirection and a code with this PR when logged in as a different user and doing the switch via <server>/index.php/apps/oauth2/logout?user=<user>&requesttoken=<token>&response_type=code&client_id=<client_id>&redirect_uri=<redirect_url> (same as described in #66 (comment)) I guess it's in the PR's scope, no?

js/login.js Outdated
var user = $("data[key='oauth2']").attr('user');
if (user) {
$('#password').val('');
$('#user').val(user);
Copy link
Contributor

Choose a reason for hiding this comment

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

@DeepDiver1975 what do you think of including:

$('#user').prop('readonly', true);

... so we force the user to login as the original user?

We'll loose focus on the login form though; password should receive it now for better UX I'm guessing.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm still getting a redirection and a code with this PR when logged in as a different user and doing the switch via /index.php/apps/oauth2/logout?user=&requesttoken=&response_type=code&client_id=<client_id>&redirect_uri=<redirect_url> (same as described in #66 (comment)) I guess it's in the PR's scope, no?

I still need to work on this ... maybe in scope .... lets see ...

@SamuAlfageme SamuAlfageme mentioned this pull request Jul 31, 2017
8 tasks
@DeepDiver1975
Copy link
Member Author

$('#user').prop('readonly', true);

... so we force the user to login as the original user?

both taken care of ....

@SamuAlfageme
Copy link
Contributor

Very nice! Working flawlessly together with owncloud/core#28511 👍

@DeepDiver1975 move #67 (review) to a different issue and merge here?

@DeepDiver1975
Copy link
Member Author

@DeepDiver1975 move #67 (review) to a different issue and merge here?

yes please - I need to understand this better ... THX

@DeepDiver1975 DeepDiver1975 merged commit e6798ed into master Aug 1, 2017
@DeepDiver1975 DeepDiver1975 deleted the set-user-on-login branch August 1, 2017 10:07
@SamuAlfageme
Copy link
Contributor

yes please - I need to understand this better ... THX

Done in #68. Hope it helps!

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.

The login page do not suggest the user despite the user= in the URL
2 participants