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.

Sign up for GitHub

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

Jump to bottom

Make embedded OAuth authentication model default for both singe and multiuser che && allow to get OAuth token with machine token #15705

Merged
merged 4 commits into from
Feb 25, 2020
Merged

Make embedded OAuth authentication model default for both singe and multiuser che && allow to get OAuth token with machine token #15705

merged 4 commits into from
Feb 25, 2020

Conversation

skabashnyuk
Copy link
Contributor

@skabashnyuk skabashnyuk commented Jan 15, 2020 •
edited
Loading

What does this PR do?

Make embedded OAuth authentication model default for both singe and multiuser che

  • set OAuth embedded mode as default for singe/multi user Che.
  • fixed dashboard, now it correctly adds user id for api/oauth/authenticate requests
  • Allowed to get OAuth tokens with machine token.

What issues does this PR fix or reference?

#15672

Release Notes

n/a

Docs PR

n/a

@skabashnyuk
Copy link
Contributor Author

CC @ibuziuk

@che-bot
Copy link
Contributor

che-bot commented Jan 15, 2020

E2E tests of Eclipse Che Multiuser on OCP has failed:

@che-bot che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/task Internal things, technical debt, and to-do tasks to be performed. labels Jan 15, 2020
@skabashnyuk skabashnyuk marked this pull request as ready for review January 20, 2020 07:51
@che-bot
Copy link
Contributor

che-bot commented Jan 25, 2020

✅ E2E Happy path tests succeed 🎉

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

@che-bot
Copy link
Contributor

che-bot commented Jan 25, 2020

E2E tests of Eclipse Che Multiuser on OCP has failed:

@skabashnyuk
Make embedded OAuth authentication model default for both singe and m…
ea43d22 
…ultiuser che

Signed-off-by: Sergii Kabashniuk <skabashniuk@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Jan 25, 2020

✅ E2E Happy path tests succeed 🎉

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

@che-bot
Copy link
Contributor

che-bot commented Jan 26, 2020

E2E tests of Eclipse Che Multiuser on OCP has failed:

@akurinnoy
Fix GitHub authentication link
03434e9 
Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Jan 27, 2020

✅ E2E Happy path tests succeed 🎉

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

@che-bot
Copy link
Contributor

che-bot commented Jan 27, 2020

E2E tests of Eclipse Che Multiuser on OCP has failed:

@skabashnyuk
Copy link
Contributor Author

I've tested it with enabled OpenshiftOAuth and Github OAuth work correctly.
CC @ibuziuk @davidfestal
openshift-github

@che-bot
Copy link
Contributor

che-bot commented Jan 28, 2020

✅ E2E Happy path tests succeed 🎉

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

@che-bot
Copy link
Contributor

che-bot commented Jan 28, 2020

E2E tests of Eclipse Che Multiuser on OCP has failed:

@skabashnyuk skabashnyuk changed the title Make embedded OAuth authentication model default for both singe and multiuser che [WIP] Make embedded OAuth authentication model default for both singe and multiuser che Jan 30, 2020
@skabashnyuk skabashnyuk changed the title [WIP] Make embedded OAuth authentication model default for both singe and multiuser che [WIP] Make embedded OAuth authentication model default for both singe and multiuser che && allow to get OAuth token with machine token Jan 30, 2020
@skabashnyuk
Copy link
Contributor Author

I have some concerns about extending the permission of the machine token. That makes sharing workspace too open and the user may not expect that if you share workspace you also make it possible to access all data that is available with machine token since we mount it as an environment variable. Here we have an open discussion about the future of sharing #15304

@davidfestal @l0rd @benoitf what do you think, is this a legitimate concern?

@che-bot
Copy link
Contributor

che-bot commented Jan 31, 2020

Can one of the admins verify this patch?

benoitf
benoitf approved these changes Feb 10, 2020
View reviewed changes
Copy link
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

AFAIK ssh keys are already mounted so this PR is not opening the door when a workspace is shared.

@vinokurig vinokurig mentioned this pull request Feb 11, 2020
Merged
@skabashnyuk skabashnyuk changed the title [WIP] Make embedded OAuth authentication model default for both singe and multiuser che && allow to get OAuth token with machine token Make embedded OAuth authentication model default for both singe and multiuser che && allow to get OAuth token with machine token Feb 21, 2020
@che-bot
Copy link
Contributor

che-bot commented Feb 21, 2020

✅ E2E Happy path tests succeed 🎉

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

@skabashnyuk skabashnyuk merged commit c88df78 into master Feb 25, 2020
@skabashnyuk skabashnyuk deleted the che15672 branch February 25, 2020 07:13
@che-bot che-bot added this to the 7.10.0 milestone Feb 25, 2020
@che-bot che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Feb 25, 2020
@skabashnyuk skabashnyuk mentioned this pull request Mar 18, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akurinnoy akurinnoy akurinnoy approved these changes

@SkorikSergey SkorikSergey SkorikSergey approved these changes

@benoitf benoitf benoitf approved these changes

@ibuziuk ibuziuk Awaiting requested review from ibuziuk

@l0rd l0rd Awaiting requested review from l0rd

@mshaposhnik mshaposhnik Awaiting requested review from mshaposhnik

@nickboldt nickboldt Awaiting requested review from nickboldt

@rhopp rhopp Awaiting requested review from rhopp

@sleshchenko sleshchenko Awaiting requested review from sleshchenko

@ashumilova ashumilova Awaiting requested review from ashumilova

Assignees
No one assigned
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed.
Projects
None yet
Milestone
7.10
Development

Successfully merging this pull request may close these issues.
5 participants
@skabashnyuk @che-bot @benoitf @SkorikSergey @akurinnoy