* [pve-devel] [PATCH pve-manager] ui: implement OpenId login
2021-06-24 8:17 [pve-devel] [PATCH pve-access-control 1/4] add OpenId configuration Dietmar Maurer
@ 2021-06-24 8:17 ` Dietmar Maurer
2021-06-29 8:13 ` Fabian Grünbichler
` (2 more replies)
2021-06-24 8:18 ` [pve-devel] [PATCH pve-access-control 2/4] depend on libpve-rs-perl Dietmar Maurer
` (3 subsequent siblings)
4 siblings, 3 replies; 11+ messages in thread
From: Dietmar Maurer @ 2021-06-24 8:17 UTC (permalink / raw)
To: pve-devel
---
PVE/HTTPServer.pm | 4 +-
www/manager6/Utils.js | 8 +++
www/manager6/window/LoginWindow.js | 105 ++++++++++++++++++++++++++++-
3 files changed, 114 insertions(+), 3 deletions(-)
diff --git a/PVE/HTTPServer.pm b/PVE/HTTPServer.pm
index 636b562b..dabdf7f3 100755
--- a/PVE/HTTPServer.pm
+++ b/PVE/HTTPServer.pm
@@ -68,7 +68,9 @@ sub auth_handler {
# explicitly allow some calls without auth
if (($rel_uri eq '/access/domains' && $method eq 'GET') ||
- ($rel_uri eq '/access/ticket' && ($method eq 'GET' || $method eq 'POST'))) {
+ ($rel_uri eq '/access/ticket' && ($method eq 'GET' || $method eq 'POST')) ||
+ ($rel_uri eq '/access/openid/login' && $method eq 'POST') ||
+ ($rel_uri eq '/access/openid/auth-url' && $method eq 'POST')) {
$require_auth = 0;
}
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 3415c9eb..c2d139f9 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1742,6 +1742,14 @@ Ext.define('PVE.Utils', {
return true;
},
+
+ openid_login_param: function() {
+ let param = Ext.Object.fromQueryString(window.location.search);
+ if (param.state !== undefined && param.code !== undefined) {
+ return param;
+ }
+ return undefined;
+ },
},
singleton: true,
diff --git a/www/manager6/window/LoginWindow.js b/www/manager6/window/LoginWindow.js
index 72078080..5d3d06b8 100644
--- a/www/manager6/window/LoginWindow.js
+++ b/www/manager6/window/LoginWindow.js
@@ -2,6 +2,21 @@
Ext.define('PVE.window.LoginWindow', {
extend: 'Ext.window.Window',
+ viewModel: {
+ data: {
+ openid: false,
+ },
+ formulas: {
+ button_text: function(get) {
+ if (get("openid") === true) {
+ return gettext("Login (OpenID redirect)");
+ } else {
+ return gettext("Login");
+ }
+ },
+ },
+ },
+
controller: {
xclass: 'Ext.app.ViewController',
@@ -18,6 +33,33 @@ Ext.define('PVE.window.LoginWindow', {
return;
}
+ let redirect_url = location.origin;
+ let params = form.getValues();
+
+ if (this.getViewModel().data.openid === true) {
+ let realm = params.realm;
+ Proxmox.Utils.API2Request({
+ url: '/api2/extjs/access/openid/auth-url',
+ params: {
+ realm: realm,
+ "redirect-url": redirect_url,
+ },
+ method: 'POST',
+ success: function(resp, opts) {
+ window.location = resp.result.data;
+ },
+ failure: function(resp, opts) {
+ Proxmox.Utils.authClear();
+ form.unmask();
+ Ext.MessageBox.alert(
+ gettext('Error'),
+ gettext('OpenId redirect failed. Please try again<br>Error: ' + resp.htmlStatus),
+ );
+ },
+ });
+ return;
+ }
+
view.el.mask(gettext('Please wait...'), 'x-mask-loading');
// set or clear username
@@ -162,11 +204,21 @@ Ext.define('PVE.window.LoginWindow', {
window.location.reload();
},
},
- 'button[reference=loginButton]': {
+ 'field[name=realm]': {
+ change: function(f, value) {
+ let record = f.store.getById(value);
+ if (record === undefined) return;
+ let data = record.data;
+ this.getViewModel().set("openid", data.type === "openid");
+ },
+ },
+ 'button[reference=loginButton]': {
click: 'onLogon',
},
'#': {
show: function() {
+ var me = this;
+
var sp = Ext.state.Manager.getProvider();
var checkboxField = this.lookupReference('saveunField');
var unField = this.lookupReference('usernameField');
@@ -180,6 +232,42 @@ Ext.define('PVE.window.LoginWindow', {
var pwField = this.lookupReference('passwordField');
pwField.focus();
}
+
+ let param = PVE.Utils.openid_login_param();
+ if (param !== undefined) {
+ Proxmox.Utils.authClear();
+
+ let loginForm = this.lookupReference('loginForm');
+ loginForm.mask(gettext('OpenID login - please wait...'), 'x-mask-loading');
+
+ let redirect_url = location.origin;
+
+ Proxmox.Utils.API2Request({
+ url: '/api2/extjs/access/openid/login',
+ params: {
+ state: param.state,
+ code: param.code,
+ "redirect-url": redirect_url,
+ },
+ method: 'POST',
+ failure: function(response) {
+ loginForm.unmask();
+ Ext.MessageBox.alert(
+ gettext('Error'),
+ gettext('Login failed. Please try again<br>Error: ' + response.htmlStatus),
+ function() {
+ window.location = redirect_url;
+ },
+ );
+ },
+ success: function(response, options) {
+ loginForm.unmask();
+ let data = response.result.data;
+ history.replaceState(null, '', redirect_url);
+ me.success(data);
+ },
+ });
+ }
},
},
},
@@ -217,6 +305,10 @@ Ext.define('PVE.window.LoginWindow', {
itemId: 'usernameField',
reference: 'usernameField',
stateId: 'login-username',
+ bind: {
+ visible: "{!openid}",
+ disabled: "{openid}",
+ },
},
{
xtype: 'textfield',
@@ -224,6 +316,10 @@ Ext.define('PVE.window.LoginWindow', {
fieldLabel: gettext('Password'),
name: 'password',
reference: 'passwordField',
+ bind: {
+ visible: "{!openid}",
+ disabled: "{openid}",
+ },
},
{
xtype: 'pmxRealmComboBox',
@@ -248,9 +344,14 @@ Ext.define('PVE.window.LoginWindow', {
labelWidth: 250,
labelAlign: 'right',
submitValue: false,
+ bind: {
+ visible: "{!openid}",
+ },
},
{
- text: gettext('Login'),
+ bind: {
+ text: "{button_text}",
+ },
reference: 'loginButton',
},
],
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [pve-devel] [PATCH pve-manager] ui: implement OpenId login
2021-06-24 8:17 ` [pve-devel] [PATCH pve-manager] ui: implement OpenId login Dietmar Maurer
@ 2021-06-29 8:13 ` Fabian Grünbichler
2021-06-29 9:15 ` Thomas Lamprecht
2021-06-29 8:28 ` Fabian Grünbichler
2021-07-02 13:13 ` [pve-devel] applied: " Thomas Lamprecht
2 siblings, 1 reply; 11+ messages in thread
From: Fabian Grünbichler @ 2021-06-29 8:13 UTC (permalink / raw)
To: Proxmox VE development discussion
not directly related to this patch - we should probably disable TFA for
openid realms (and their users), since TFA would need to be handled at
the openid provider in that case.. e.g., if I login via openid and then
hit TFA in the top right corner user menu, I get prompted for a password
to setup TFA which is of course not possible ;)
On June 24, 2021 10:17 am, Dietmar Maurer wrote:
> ---
> PVE/HTTPServer.pm | 4 +-
> www/manager6/Utils.js | 8 +++
> www/manager6/window/LoginWindow.js | 105 ++++++++++++++++++++++++++++-
> 3 files changed, 114 insertions(+), 3 deletions(-)
>
> diff --git a/PVE/HTTPServer.pm b/PVE/HTTPServer.pm
> index 636b562b..dabdf7f3 100755
> --- a/PVE/HTTPServer.pm
> +++ b/PVE/HTTPServer.pm
> @@ -68,7 +68,9 @@ sub auth_handler {
>
> # explicitly allow some calls without auth
> if (($rel_uri eq '/access/domains' && $method eq 'GET') ||
> - ($rel_uri eq '/access/ticket' && ($method eq 'GET' || $method eq 'POST'))) {
> + ($rel_uri eq '/access/ticket' && ($method eq 'GET' || $method eq 'POST')) ||
> + ($rel_uri eq '/access/openid/login' && $method eq 'POST') ||
> + ($rel_uri eq '/access/openid/auth-url' && $method eq 'POST')) {
> $require_auth = 0;
> }
>
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index 3415c9eb..c2d139f9 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -1742,6 +1742,14 @@ Ext.define('PVE.Utils', {
>
> return true;
> },
> +
> + openid_login_param: function() {
> + let param = Ext.Object.fromQueryString(window.location.search);
> + if (param.state !== undefined && param.code !== undefined) {
> + return param;
> + }
> + return undefined;
> + },
> },
>
> singleton: true,
> diff --git a/www/manager6/window/LoginWindow.js b/www/manager6/window/LoginWindow.js
> index 72078080..5d3d06b8 100644
> --- a/www/manager6/window/LoginWindow.js
> +++ b/www/manager6/window/LoginWindow.js
> @@ -2,6 +2,21 @@
> Ext.define('PVE.window.LoginWindow', {
> extend: 'Ext.window.Window',
>
> + viewModel: {
> + data: {
> + openid: false,
> + },
> + formulas: {
> + button_text: function(get) {
> + if (get("openid") === true) {
> + return gettext("Login (OpenID redirect)");
> + } else {
> + return gettext("Login");
> + }
> + },
> + },
> + },
> +
> controller: {
>
> xclass: 'Ext.app.ViewController',
> @@ -18,6 +33,33 @@ Ext.define('PVE.window.LoginWindow', {
> return;
> }
>
> + let redirect_url = location.origin;
> + let params = form.getValues();
> +
> + if (this.getViewModel().data.openid === true) {
> + let realm = params.realm;
> + Proxmox.Utils.API2Request({
> + url: '/api2/extjs/access/openid/auth-url',
> + params: {
> + realm: realm,
> + "redirect-url": redirect_url,
> + },
> + method: 'POST',
> + success: function(resp, opts) {
> + window.location = resp.result.data;
> + },
> + failure: function(resp, opts) {
> + Proxmox.Utils.authClear();
> + form.unmask();
> + Ext.MessageBox.alert(
> + gettext('Error'),
> + gettext('OpenId redirect failed. Please try again<br>Error: ' + resp.htmlStatus),
> + );
> + },
> + });
> + return;
> + }
> +
> view.el.mask(gettext('Please wait...'), 'x-mask-loading');
>
> // set or clear username
> @@ -162,11 +204,21 @@ Ext.define('PVE.window.LoginWindow', {
> window.location.reload();
> },
> },
> - 'button[reference=loginButton]': {
> + 'field[name=realm]': {
> + change: function(f, value) {
> + let record = f.store.getById(value);
> + if (record === undefined) return;
> + let data = record.data;
> + this.getViewModel().set("openid", data.type === "openid");
> + },
> + },
> + 'button[reference=loginButton]': {
> click: 'onLogon',
> },
> '#': {
> show: function() {
> + var me = this;
> +
> var sp = Ext.state.Manager.getProvider();
> var checkboxField = this.lookupReference('saveunField');
> var unField = this.lookupReference('usernameField');
> @@ -180,6 +232,42 @@ Ext.define('PVE.window.LoginWindow', {
> var pwField = this.lookupReference('passwordField');
> pwField.focus();
> }
> +
> + let param = PVE.Utils.openid_login_param();
> + if (param !== undefined) {
> + Proxmox.Utils.authClear();
> +
> + let loginForm = this.lookupReference('loginForm');
> + loginForm.mask(gettext('OpenID login - please wait...'), 'x-mask-loading');
> +
> + let redirect_url = location.origin;
> +
> + Proxmox.Utils.API2Request({
> + url: '/api2/extjs/access/openid/login',
> + params: {
> + state: param.state,
> + code: param.code,
> + "redirect-url": redirect_url,
> + },
> + method: 'POST',
> + failure: function(response) {
> + loginForm.unmask();
> + Ext.MessageBox.alert(
> + gettext('Error'),
> + gettext('Login failed. Please try again<br>Error: ' + response.htmlStatus),
> + function() {
> + window.location = redirect_url;
> + },
> + );
> + },
> + success: function(response, options) {
> + loginForm.unmask();
> + let data = response.result.data;
> + history.replaceState(null, '', redirect_url);
> + me.success(data);
> + },
> + });
> + }
> },
> },
> },
> @@ -217,6 +305,10 @@ Ext.define('PVE.window.LoginWindow', {
> itemId: 'usernameField',
> reference: 'usernameField',
> stateId: 'login-username',
> + bind: {
> + visible: "{!openid}",
> + disabled: "{openid}",
> + },
> },
> {
> xtype: 'textfield',
> @@ -224,6 +316,10 @@ Ext.define('PVE.window.LoginWindow', {
> fieldLabel: gettext('Password'),
> name: 'password',
> reference: 'passwordField',
> + bind: {
> + visible: "{!openid}",
> + disabled: "{openid}",
> + },
> },
> {
> xtype: 'pmxRealmComboBox',
> @@ -248,9 +344,14 @@ Ext.define('PVE.window.LoginWindow', {
> labelWidth: 250,
> labelAlign: 'right',
> submitValue: false,
> + bind: {
> + visible: "{!openid}",
> + },
> },
> {
> - text: gettext('Login'),
> + bind: {
> + text: "{button_text}",
> + },
> reference: 'loginButton',
> },
> ],
> --
> 2.30.2
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [pve-devel] [PATCH pve-manager] ui: implement OpenId login
2021-06-29 8:13 ` Fabian Grünbichler
@ 2021-06-29 9:15 ` Thomas Lamprecht
0 siblings, 0 replies; 11+ messages in thread
From: Thomas Lamprecht @ 2021-06-29 9:15 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
On 29.06.21 10:13, Fabian Grünbichler wrote:
> not directly related to this patch - we should probably disable TFA for
> openid realms (and their users), since TFA would need to be handled at
> the openid provider in that case.. e.g., if I login via openid and then
> hit TFA in the top right corner user menu, I get prompted for a password
> to setup TFA which is of course not possible ;)
Yes true, both TFA and probably also "edit password" would be disabled,
Dietmar mentioned the lack of that already in an off-list talk last week
to me.
The realm add/edit window you mentioned in your other reply shouldn't be
that hard, Dominik can probably do that in an hour, but yes certainly wanted
for full inclusion.
So agreeing on both counts, just mentioning that there was awareness about
those two points, and both should be addressable with relatively low effort.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pve-devel] [PATCH pve-manager] ui: implement OpenId login
2021-06-24 8:17 ` [pve-devel] [PATCH pve-manager] ui: implement OpenId login Dietmar Maurer
2021-06-29 8:13 ` Fabian Grünbichler
@ 2021-06-29 8:28 ` Fabian Grünbichler
2021-07-02 13:13 ` [pve-devel] applied: " Thomas Lamprecht
2 siblings, 0 replies; 11+ messages in thread
From: Fabian Grünbichler @ 2021-06-29 8:28 UTC (permalink / raw)
To: Proxmox VE development discussion
also missing in pve-manager - code to add/edit openid realms via the
GUI..
On June 24, 2021 10:17 am, Dietmar Maurer wrote:
> ---
> PVE/HTTPServer.pm | 4 +-
> www/manager6/Utils.js | 8 +++
> www/manager6/window/LoginWindow.js | 105 ++++++++++++++++++++++++++++-
> 3 files changed, 114 insertions(+), 3 deletions(-)
>
> diff --git a/PVE/HTTPServer.pm b/PVE/HTTPServer.pm
> index 636b562b..dabdf7f3 100755
> --- a/PVE/HTTPServer.pm
> +++ b/PVE/HTTPServer.pm
> @@ -68,7 +68,9 @@ sub auth_handler {
>
> # explicitly allow some calls without auth
> if (($rel_uri eq '/access/domains' && $method eq 'GET') ||
> - ($rel_uri eq '/access/ticket' && ($method eq 'GET' || $method eq 'POST'))) {
> + ($rel_uri eq '/access/ticket' && ($method eq 'GET' || $method eq 'POST')) ||
> + ($rel_uri eq '/access/openid/login' && $method eq 'POST') ||
> + ($rel_uri eq '/access/openid/auth-url' && $method eq 'POST')) {
> $require_auth = 0;
> }
>
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index 3415c9eb..c2d139f9 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -1742,6 +1742,14 @@ Ext.define('PVE.Utils', {
>
> return true;
> },
> +
> + openid_login_param: function() {
> + let param = Ext.Object.fromQueryString(window.location.search);
> + if (param.state !== undefined && param.code !== undefined) {
> + return param;
> + }
> + return undefined;
> + },
> },
>
> singleton: true,
> diff --git a/www/manager6/window/LoginWindow.js b/www/manager6/window/LoginWindow.js
> index 72078080..5d3d06b8 100644
> --- a/www/manager6/window/LoginWindow.js
> +++ b/www/manager6/window/LoginWindow.js
> @@ -2,6 +2,21 @@
> Ext.define('PVE.window.LoginWindow', {
> extend: 'Ext.window.Window',
>
> + viewModel: {
> + data: {
> + openid: false,
> + },
> + formulas: {
> + button_text: function(get) {
> + if (get("openid") === true) {
> + return gettext("Login (OpenID redirect)");
> + } else {
> + return gettext("Login");
> + }
> + },
> + },
> + },
> +
> controller: {
>
> xclass: 'Ext.app.ViewController',
> @@ -18,6 +33,33 @@ Ext.define('PVE.window.LoginWindow', {
> return;
> }
>
> + let redirect_url = location.origin;
> + let params = form.getValues();
> +
> + if (this.getViewModel().data.openid === true) {
> + let realm = params.realm;
> + Proxmox.Utils.API2Request({
> + url: '/api2/extjs/access/openid/auth-url',
> + params: {
> + realm: realm,
> + "redirect-url": redirect_url,
> + },
> + method: 'POST',
> + success: function(resp, opts) {
> + window.location = resp.result.data;
> + },
> + failure: function(resp, opts) {
> + Proxmox.Utils.authClear();
> + form.unmask();
> + Ext.MessageBox.alert(
> + gettext('Error'),
> + gettext('OpenId redirect failed. Please try again<br>Error: ' + resp.htmlStatus),
> + );
> + },
> + });
> + return;
> + }
> +
> view.el.mask(gettext('Please wait...'), 'x-mask-loading');
>
> // set or clear username
> @@ -162,11 +204,21 @@ Ext.define('PVE.window.LoginWindow', {
> window.location.reload();
> },
> },
> - 'button[reference=loginButton]': {
> + 'field[name=realm]': {
> + change: function(f, value) {
> + let record = f.store.getById(value);
> + if (record === undefined) return;
> + let data = record.data;
> + this.getViewModel().set("openid", data.type === "openid");
> + },
> + },
> + 'button[reference=loginButton]': {
> click: 'onLogon',
> },
> '#': {
> show: function() {
> + var me = this;
> +
> var sp = Ext.state.Manager.getProvider();
> var checkboxField = this.lookupReference('saveunField');
> var unField = this.lookupReference('usernameField');
> @@ -180,6 +232,42 @@ Ext.define('PVE.window.LoginWindow', {
> var pwField = this.lookupReference('passwordField');
> pwField.focus();
> }
> +
> + let param = PVE.Utils.openid_login_param();
> + if (param !== undefined) {
> + Proxmox.Utils.authClear();
> +
> + let loginForm = this.lookupReference('loginForm');
> + loginForm.mask(gettext('OpenID login - please wait...'), 'x-mask-loading');
> +
> + let redirect_url = location.origin;
> +
> + Proxmox.Utils.API2Request({
> + url: '/api2/extjs/access/openid/login',
> + params: {
> + state: param.state,
> + code: param.code,
> + "redirect-url": redirect_url,
> + },
> + method: 'POST',
> + failure: function(response) {
> + loginForm.unmask();
> + Ext.MessageBox.alert(
> + gettext('Error'),
> + gettext('Login failed. Please try again<br>Error: ' + response.htmlStatus),
> + function() {
> + window.location = redirect_url;
> + },
> + );
> + },
> + success: function(response, options) {
> + loginForm.unmask();
> + let data = response.result.data;
> + history.replaceState(null, '', redirect_url);
> + me.success(data);
> + },
> + });
> + }
> },
> },
> },
> @@ -217,6 +305,10 @@ Ext.define('PVE.window.LoginWindow', {
> itemId: 'usernameField',
> reference: 'usernameField',
> stateId: 'login-username',
> + bind: {
> + visible: "{!openid}",
> + disabled: "{openid}",
> + },
> },
> {
> xtype: 'textfield',
> @@ -224,6 +316,10 @@ Ext.define('PVE.window.LoginWindow', {
> fieldLabel: gettext('Password'),
> name: 'password',
> reference: 'passwordField',
> + bind: {
> + visible: "{!openid}",
> + disabled: "{openid}",
> + },
> },
> {
> xtype: 'pmxRealmComboBox',
> @@ -248,9 +344,14 @@ Ext.define('PVE.window.LoginWindow', {
> labelWidth: 250,
> labelAlign: 'right',
> submitValue: false,
> + bind: {
> + visible: "{!openid}",
> + },
> },
> {
> - text: gettext('Login'),
> + bind: {
> + text: "{button_text}",
> + },
> reference: 'loginButton',
> },
> ],
> --
> 2.30.2
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* [pve-devel] applied: [PATCH pve-manager] ui: implement OpenId login
2021-06-24 8:17 ` [pve-devel] [PATCH pve-manager] ui: implement OpenId login Dietmar Maurer
2021-06-29 8:13 ` Fabian Grünbichler
2021-06-29 8:28 ` Fabian Grünbichler
@ 2021-07-02 13:13 ` Thomas Lamprecht
2 siblings, 0 replies; 11+ messages in thread
From: Thomas Lamprecht @ 2021-07-02 13:13 UTC (permalink / raw)
To: Proxmox VE development discussion, Dietmar Maurer
On 24.06.21 10:17, Dietmar Maurer wrote:
> ---
> PVE/HTTPServer.pm | 4 +-
> www/manager6/Utils.js | 8 +++
> www/manager6/window/LoginWindow.js | 105 ++++++++++++++++++++++++++++-
> 3 files changed, 114 insertions(+), 3 deletions(-)
>
>
applied, thanks!
just FYI:
I squashed in a few fixes though, for one the gettext usage was broken as
things like `gettext('' + variable)` cannot work (would never wind a translation)
and the scanner in proxmox-i18n cannot detect such things either.
One must use either `Ext.String.format((gettext("bla bla: {0}"), variable)` or add
the variable after the gettext call, depending on use-case.
Also factored out the `openid_login_param` helper to proxmox-widget toolkit
as `getOpenIDRedirectionAuthorization`, also fixed some casing (we prefer camelCase
over snake_case in JS for new code[0]).
[0]: https://pve.proxmox.com/wiki/Javascript_Style_Guide#Casing
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pve-devel] [PATCH pve-access-control 2/4] depend on libpve-rs-perl
2021-06-24 8:17 [pve-devel] [PATCH pve-access-control 1/4] add OpenId configuration Dietmar Maurer
2021-06-24 8:17 ` [pve-devel] [PATCH pve-manager] ui: implement OpenId login Dietmar Maurer
@ 2021-06-24 8:18 ` Dietmar Maurer
2021-06-24 8:18 ` [pve-devel] [PATCH pve-access-control 3/4] api: implement openid API Dietmar Maurer
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Dietmar Maurer @ 2021-06-24 8:18 UTC (permalink / raw)
To: pve-devel
---
debian/control | 2 ++
1 file changed, 2 insertions(+)
diff --git a/debian/control b/debian/control
index 81a32bd..3ef748b 100644
--- a/debian/control
+++ b/debian/control
@@ -10,6 +10,7 @@ Build-Depends: debhelper (>= 12~),
lintian,
perl,
libpve-cluster-perl,
+ libpve-rs-perl,
pve-cluster (>= 6.1-4),
pve-doc-generator (>= 5.3-3),
Standards-Version: 4.5.1
@@ -27,6 +28,7 @@ Depends: libauthen-pam-perl,
libnet-ssleay-perl,
libpve-common-perl (>= 6.0-18),
libpve-cluster-perl,
+ libpve-rs-perl,
libpve-u2f-server-perl (>= 1.0-2),
libuuid-perl,
pve-cluster (>= 6.1-4),
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [pve-devel] [PATCH pve-access-control 3/4] api: implement openid API
2021-06-24 8:17 [pve-devel] [PATCH pve-access-control 1/4] add OpenId configuration Dietmar Maurer
2021-06-24 8:17 ` [pve-devel] [PATCH pve-manager] ui: implement OpenId login Dietmar Maurer
2021-06-24 8:18 ` [pve-devel] [PATCH pve-access-control 2/4] depend on libpve-rs-perl Dietmar Maurer
@ 2021-06-24 8:18 ` Dietmar Maurer
2021-06-29 8:22 ` Fabian Grünbichler
2021-06-24 8:18 ` [pve-devel] [PATCH pve-access-control 4/4] implement OpenID autocreate user feature Dietmar Maurer
2021-06-29 8:29 ` [pve-devel] [PATCH pve-access-control 1/4] add OpenId configuration Fabian Grünbichler
4 siblings, 1 reply; 11+ messages in thread
From: Dietmar Maurer @ 2021-06-24 8:18 UTC (permalink / raw)
To: pve-devel
This moves compute_api_permission() into RPCEnvironment.pm.
---
src/PVE/API2/AccessControl.pm | 60 ++--------
src/PVE/API2/Makefile | 3 +-
src/PVE/API2/OpenId.pm | 214 ++++++++++++++++++++++++++++++++++
src/PVE/RPCEnvironment.pm | 49 ++++++++
4 files changed, 273 insertions(+), 53 deletions(-)
create mode 100644 src/PVE/API2/OpenId.pm
diff --git a/src/PVE/API2/AccessControl.pm b/src/PVE/API2/AccessControl.pm
index a77694b..6dec66c 100644
--- a/src/PVE/API2/AccessControl.pm
+++ b/src/PVE/API2/AccessControl.pm
@@ -19,9 +19,9 @@ use PVE::API2::User;
use PVE::API2::Group;
use PVE::API2::Role;
use PVE::API2::ACL;
+use PVE::API2::OpenId;
use PVE::Auth::Plugin;
use PVE::OTP;
-use PVE::Tools;
my $u2f_available = 0;
eval {
@@ -56,6 +56,11 @@ __PACKAGE__->register_method ({
path => 'domains',
});
+__PACKAGE__->register_method ({
+ subclass => "PVE::API2::OpenId",
+ path => 'openid',
+});
+
__PACKAGE__->register_method ({
name => 'index',
path => '',
@@ -165,55 +170,6 @@ my $create_ticket = sub {
};
};
-my $compute_api_permission = sub {
- my ($rpcenv, $authuser) = @_;
-
- my $usercfg = $rpcenv->{user_cfg};
-
- my $res = {};
- my $priv_re_map = {
- vms => qr/VM\.|Permissions\.Modify/,
- access => qr/(User|Group)\.|Permissions\.Modify/,
- storage => qr/Datastore\.|Permissions\.Modify/,
- nodes => qr/Sys\.|Permissions\.Modify/,
- sdn => qr/SDN\.|Permissions\.Modify/,
- dc => qr/Sys\.Audit|SDN\./,
- };
- map { $res->{$_} = {} } keys %$priv_re_map;
-
- my $required_paths = ['/', '/nodes', '/access/groups', '/vms', '/storage', '/sdn'];
-
- my $checked_paths = {};
- foreach my $path (@$required_paths, keys %{$usercfg->{acl}}) {
- next if $checked_paths->{$path};
- $checked_paths->{$path} = 1;
-
- my $path_perm = $rpcenv->permissions($authuser, $path);
-
- my $toplevel = ($path =~ /^\/(\w+)/) ? $1 : 'dc';
- if ($toplevel eq 'pool') {
- foreach my $priv (keys %$path_perm) {
- if ($priv =~ m/^VM\./) {
- $res->{vms}->{$priv} = 1;
- } elsif ($priv =~ m/^Datastore\./) {
- $res->{storage}->{$priv} = 1;
- } elsif ($priv eq 'Permissions.Modify') {
- $res->{storage}->{$priv} = 1;
- $res->{vms}->{$priv} = 1;
- }
- }
- } else {
- my $priv_regex = $priv_re_map->{$toplevel} // next;
- foreach my $priv (keys %$path_perm) {
- next if $priv !~ m/^($priv_regex)/;
- $res->{$toplevel}->{$priv} = 1;
- }
- }
- }
-
- return $res;
-};
-
__PACKAGE__->register_method ({
name => 'get_ticket',
path => 'ticket',
@@ -314,7 +270,7 @@ __PACKAGE__->register_method ({
die PVE::Exception->new("authentication failure\n", code => 401);
}
- $res->{cap} = &$compute_api_permission($rpcenv, $username)
+ $res->{cap} = $rpcenv->compute_api_permission($username)
if !defined($res->{NeedTFA});
my $clinfo = PVE::Cluster::get_clinfo();
@@ -659,7 +615,7 @@ __PACKAGE__->register_method({
return {
ticket => PVE::AccessControl::assemble_ticket($authuser),
- cap => &$compute_api_permission($rpcenv, $authuser),
+ cap => $rpcenv->compute_api_permission($authuser),
}
}});
diff --git a/src/PVE/API2/Makefile b/src/PVE/API2/Makefile
index 1bf8c05..4e49037 100644
--- a/src/PVE/API2/Makefile
+++ b/src/PVE/API2/Makefile
@@ -5,7 +5,8 @@ API2_SOURCES= \
ACL.pm \
Role.pm \
Group.pm \
- User.pm
+ User.pm \
+ OpenId.pm
.PHONY: install
install:
diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
new file mode 100644
index 0000000..db9f9eb
--- /dev/null
+++ b/src/PVE/API2/OpenId.pm
@@ -0,0 +1,214 @@
+package PVE::API2::OpenId;
+
+use strict;
+use warnings;
+
+use PVE::Tools qw(extract_param);
+use PVE::RS::OpenId;
+
+use PVE::Exception qw(raise raise_perm_exc raise_param_exc);
+use PVE::SafeSyslog;
+use PVE::RPCEnvironment;
+use PVE::Cluster qw(cfs_read_file);
+use PVE::AccessControl;
+use PVE::JSONSchema qw(get_standard_option);
+
+use PVE::RESTHandler;
+
+use base qw(PVE::RESTHandler);
+
+my $openid_state_path = "/var/lib/pve-manager";
+
+my $lookup_openid_auth = sub {
+ my ($realm, $redirect_url) = @_;
+
+ my $cfg = cfs_read_file('domains.cfg');
+ my $ids = $cfg->{ids};
+
+ die "authentication domain '$realm' does not exist\n" if !$ids->{$realm};
+
+ my $config = $ids->{$realm};
+ die "wrong realm type ($config->{type} != openid)" if $config->{type} ne "openid";
+
+ my $openid_config = {
+ issuer_url => $config->{'issuer-url'},
+ client_id => $config->{'client-id'},
+ client_key => $config->{'client-key'},
+ };
+
+ my $openid = PVE::RS::OpenId->discover($openid_config, $redirect_url);
+ return ($config, $openid);
+};
+
+__PACKAGE__->register_method ({
+ name => 'index',
+ path => '',
+ method => 'GET',
+ description => "Directory index.",
+ permissions => {
+ user => 'all',
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => {},
+ },
+ returns => {
+ type => 'array',
+ items => {
+ type => "object",
+ properties => {
+ subdir => { type => 'string' },
+ },
+ },
+ links => [ { rel => 'child', href => "{subdir}" } ],
+ },
+ code => sub {
+ my ($param) = @_;
+
+ return [
+ { subdir => 'auth-url' },
+ { subdir => 'login' },
+ ];
+ }});
+
+__PACKAGE__->register_method ({
+ name => 'auth_url',
+ path => 'auth-url',
+ method => 'POST',
+ protected => 1,
+ description => "Get the OpenId Authorization Url for the specified realm.",
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ realm => get_standard_option('realm'),
+ 'redirect-url' => {
+ description => "Redirection Url. The client should set this to the used server url (location.origin).",
+ type => 'string',
+ maxLength => 255,
+ },
+ },
+ },
+ returns => {
+ type => "string",
+ description => "Redirection URL.",
+ },
+ permissions => { user => 'world' },
+ code => sub {
+ my ($param) = @_;
+
+ my $realm = extract_param($param, 'realm');
+ my $redirect_url = extract_param($param, 'redirect-url');
+
+ my ($config, $openid) = $lookup_openid_auth->($realm, $redirect_url);
+ my $url = $openid->authorize_url($openid_state_path , $realm);
+
+ return $url;
+ }});
+
+__PACKAGE__->register_method ({
+ name => 'login',
+ path => 'login',
+ method => 'POST',
+ protected => 1,
+ description => " Verify OpenID authorization code and create a ticket.",
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ 'state' => {
+ description => "OpenId state.",
+ type => 'string',
+ maxLength => 1024,
+ },
+ code => {
+ description => "OpenId authorization code.",
+ type => 'string',
+ maxLength => 1024,
+ },
+ 'redirect-url' => {
+ description => "Redirection Url. The client should set this to the used server url (location.origin).",
+ type => 'string',
+ maxLength => 255,
+ },
+ },
+ },
+ returns => {
+ properties => {
+ username => { type => 'string' },
+ ticket => { type => 'string' },
+ CSRFPreventionToken => { type => 'string' },
+ cap => { type => 'object' }, # computed api permissions
+ clustername => { type => 'string', optional => 1 },
+ },
+ },
+ permissions => { user => 'world' },
+ code => sub {
+ my ($param) = @_;
+
+ my $rpcenv = PVE::RPCEnvironment::get();
+
+ my $res;
+ eval {
+ use Data::Dumper;
+
+ # fixme /tmp
+ my ($realm, $private_auth_state) = PVE::RS::OpenId::verify_public_auth_state(
+ $openid_state_path, $param->{'state'});
+
+ my $redirect_url = extract_param($param, 'redirect-url');
+
+ my ($config, $openid) = $lookup_openid_auth->($realm, $redirect_url);
+
+ my $info = $openid->verify_authorization_code($param->{code}, $private_auth_state);
+ my $subject = $info->{'sub'};
+
+ die "missing openid claim 'sub'" if !defined($subject);
+
+ my $unique_name = $subject; # default
+ if (defined(my $user_attr = $config->{'user-attr'})) {
+ if ($user_attr eq 'subject') {
+ $unique_name = $subject;
+ } elsif ($user_attr eq 'username') {
+ my $username = $info->{'preferred_username'};
+ die "missing claim 'preferred_username'" if !defined($username);
+ $unique_name = $username;
+ } elsif ($user_attr eq 'email') {
+ my $email = $info->{'email'};
+ die "missing claim 'email'" if !defined($email);
+ $unique_name = $email;
+ } else {
+ die "got unexpected value for user_attr '${user_attr}'";
+ }
+ }
+
+ my $username = "${unique_name}\@${realm}";
+
+ # test if user exists and is enabled
+ $rpcenv->check_user_enabled($username);
+
+ my $ticket = PVE::AccessControl::assemble_ticket($username);
+ my $csrftoken = PVE::AccessControl::assemble_csrf_prevention_token($username);
+ my $cap = $rpcenv->compute_api_permission($username);
+
+ $res = {
+ ticket => $ticket,
+ username => $username,
+ CSRFPreventionToken => $csrftoken,
+ cap => $cap,
+ };
+
+ my $clinfo = PVE::Cluster::get_clinfo();
+ if ($clinfo->{cluster}->{name} && $rpcenv->check($username, '/', ['Sys.Audit'], 1)) {
+ $res->{clustername} = $clinfo->{cluster}->{name};
+ }
+ };
+ if (my $err = $@) {
+ my $clientip = $rpcenv->get_client_ip() || '';
+ syslog('err', "openid authentication failure; rhost=$clientip msg=$err");
+ # do not return any info to prevent user enumeration attacks
+ die PVE::Exception->new("authentication failure\n", code => 401);
+ }
+
+ PVE::Cluster::log_msg('info', 'root@pam', "successful openid auth for user '$res->{username}'");
+
+ return $res;
+ }});
diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm
index 2e5a33b..8aae094 100644
--- a/src/PVE/RPCEnvironment.pm
+++ b/src/PVE/RPCEnvironment.pm
@@ -126,6 +126,55 @@ sub permissions {
return &$compile_acl_path($self, $user, $path);
}
+sub compute_api_permission {
+ my ($self, $authuser) = @_;
+
+ my $usercfg = $self->{user_cfg};
+
+ my $res = {};
+ my $priv_re_map = {
+ vms => qr/VM\.|Permissions\.Modify/,
+ access => qr/(User|Group)\.|Permissions\.Modify/,
+ storage => qr/Datastore\.|Permissions\.Modify/,
+ nodes => qr/Sys\.|Permissions\.Modify/,
+ sdn => qr/SDN\.|Permissions\.Modify/,
+ dc => qr/Sys\.Audit|SDN\./,
+ };
+ map { $res->{$_} = {} } keys %$priv_re_map;
+
+ my $required_paths = ['/', '/nodes', '/access/groups', '/vms', '/storage', '/sdn'];
+
+ my $checked_paths = {};
+ foreach my $path (@$required_paths, keys %{$usercfg->{acl}}) {
+ next if $checked_paths->{$path};
+ $checked_paths->{$path} = 1;
+
+ my $path_perm = $self->permissions($authuser, $path);
+
+ my $toplevel = ($path =~ /^\/(\w+)/) ? $1 : 'dc';
+ if ($toplevel eq 'pool') {
+ foreach my $priv (keys %$path_perm) {
+ if ($priv =~ m/^VM\./) {
+ $res->{vms}->{$priv} = 1;
+ } elsif ($priv =~ m/^Datastore\./) {
+ $res->{storage}->{$priv} = 1;
+ } elsif ($priv eq 'Permissions.Modify') {
+ $res->{storage}->{$priv} = 1;
+ $res->{vms}->{$priv} = 1;
+ }
+ }
+ } else {
+ my $priv_regex = $priv_re_map->{$toplevel} // next;
+ foreach my $priv (keys %$path_perm) {
+ next if $priv !~ m/^($priv_regex)/;
+ $res->{$toplevel}->{$priv} = 1;
+ }
+ }
+ }
+
+ return $res;
+}
+
sub get_effective_permissions {
my ($self, $user) = @_;
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [pve-devel] [PATCH pve-access-control 3/4] api: implement openid API
2021-06-24 8:18 ` [pve-devel] [PATCH pve-access-control 3/4] api: implement openid API Dietmar Maurer
@ 2021-06-29 8:22 ` Fabian Grünbichler
0 siblings, 0 replies; 11+ messages in thread
From: Fabian Grünbichler @ 2021-06-29 8:22 UTC (permalink / raw)
To: Proxmox VE development discussion
On June 24, 2021 10:18 am, Dietmar Maurer wrote:
> This moves compute_api_permission() into RPCEnvironment.pm.
> ---
> src/PVE/API2/AccessControl.pm | 60 ++--------
> src/PVE/API2/Makefile | 3 +-
> src/PVE/API2/OpenId.pm | 214 ++++++++++++++++++++++++++++++++++
> src/PVE/RPCEnvironment.pm | 49 ++++++++
> 4 files changed, 273 insertions(+), 53 deletions(-)
> create mode 100644 src/PVE/API2/OpenId.pm
>
> diff --git a/src/PVE/API2/AccessControl.pm b/src/PVE/API2/AccessControl.pm
> index a77694b..6dec66c 100644
> --- a/src/PVE/API2/AccessControl.pm
> +++ b/src/PVE/API2/AccessControl.pm
> @@ -19,9 +19,9 @@ use PVE::API2::User;
> use PVE::API2::Group;
> use PVE::API2::Role;
> use PVE::API2::ACL;
> +use PVE::API2::OpenId;
> use PVE::Auth::Plugin;
> use PVE::OTP;
> -use PVE::Tools;
>
> my $u2f_available = 0;
> eval {
> @@ -56,6 +56,11 @@ __PACKAGE__->register_method ({
> path => 'domains',
> });
>
> +__PACKAGE__->register_method ({
> + subclass => "PVE::API2::OpenId",
> + path => 'openid',
> +});
> +
> __PACKAGE__->register_method ({
> name => 'index',
> path => '',
> @@ -165,55 +170,6 @@ my $create_ticket = sub {
> };
> };
>
> -my $compute_api_permission = sub {
> - my ($rpcenv, $authuser) = @_;
> -
> - my $usercfg = $rpcenv->{user_cfg};
> -
> - my $res = {};
> - my $priv_re_map = {
> - vms => qr/VM\.|Permissions\.Modify/,
> - access => qr/(User|Group)\.|Permissions\.Modify/,
> - storage => qr/Datastore\.|Permissions\.Modify/,
> - nodes => qr/Sys\.|Permissions\.Modify/,
> - sdn => qr/SDN\.|Permissions\.Modify/,
> - dc => qr/Sys\.Audit|SDN\./,
> - };
> - map { $res->{$_} = {} } keys %$priv_re_map;
> -
> - my $required_paths = ['/', '/nodes', '/access/groups', '/vms', '/storage', '/sdn'];
> -
> - my $checked_paths = {};
> - foreach my $path (@$required_paths, keys %{$usercfg->{acl}}) {
> - next if $checked_paths->{$path};
> - $checked_paths->{$path} = 1;
> -
> - my $path_perm = $rpcenv->permissions($authuser, $path);
> -
> - my $toplevel = ($path =~ /^\/(\w+)/) ? $1 : 'dc';
> - if ($toplevel eq 'pool') {
> - foreach my $priv (keys %$path_perm) {
> - if ($priv =~ m/^VM\./) {
> - $res->{vms}->{$priv} = 1;
> - } elsif ($priv =~ m/^Datastore\./) {
> - $res->{storage}->{$priv} = 1;
> - } elsif ($priv eq 'Permissions.Modify') {
> - $res->{storage}->{$priv} = 1;
> - $res->{vms}->{$priv} = 1;
> - }
> - }
> - } else {
> - my $priv_regex = $priv_re_map->{$toplevel} // next;
> - foreach my $priv (keys %$path_perm) {
> - next if $priv !~ m/^($priv_regex)/;
> - $res->{$toplevel}->{$priv} = 1;
> - }
> - }
> - }
> -
> - return $res;
> -};
> -
> __PACKAGE__->register_method ({
> name => 'get_ticket',
> path => 'ticket',
> @@ -314,7 +270,7 @@ __PACKAGE__->register_method ({
> die PVE::Exception->new("authentication failure\n", code => 401);
> }
>
> - $res->{cap} = &$compute_api_permission($rpcenv, $username)
> + $res->{cap} = $rpcenv->compute_api_permission($username)
> if !defined($res->{NeedTFA});
>
> my $clinfo = PVE::Cluster::get_clinfo();
> @@ -659,7 +615,7 @@ __PACKAGE__->register_method({
>
> return {
> ticket => PVE::AccessControl::assemble_ticket($authuser),
> - cap => &$compute_api_permission($rpcenv, $authuser),
> + cap => $rpcenv->compute_api_permission($authuser),
> }
> }});
>
> diff --git a/src/PVE/API2/Makefile b/src/PVE/API2/Makefile
> index 1bf8c05..4e49037 100644
> --- a/src/PVE/API2/Makefile
> +++ b/src/PVE/API2/Makefile
> @@ -5,7 +5,8 @@ API2_SOURCES= \
> ACL.pm \
> Role.pm \
> Group.pm \
> - User.pm
> + User.pm \
> + OpenId.pm
>
> .PHONY: install
> install:
> diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
> new file mode 100644
> index 0000000..db9f9eb
> --- /dev/null
> +++ b/src/PVE/API2/OpenId.pm
> @@ -0,0 +1,214 @@
> +package PVE::API2::OpenId;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Tools qw(extract_param);
> +use PVE::RS::OpenId;
> +
> +use PVE::Exception qw(raise raise_perm_exc raise_param_exc);
> +use PVE::SafeSyslog;
> +use PVE::RPCEnvironment;
> +use PVE::Cluster qw(cfs_read_file);
> +use PVE::AccessControl;
> +use PVE::JSONSchema qw(get_standard_option);
> +
> +use PVE::RESTHandler;
> +
> +use base qw(PVE::RESTHandler);
> +
> +my $openid_state_path = "/var/lib/pve-manager";
> +
> +my $lookup_openid_auth = sub {
> + my ($realm, $redirect_url) = @_;
> +
> + my $cfg = cfs_read_file('domains.cfg');
> + my $ids = $cfg->{ids};
> +
> + die "authentication domain '$realm' does not exist\n" if !$ids->{$realm};
> +
> + my $config = $ids->{$realm};
> + die "wrong realm type ($config->{type} != openid)" if $config->{type} ne "openid";
missing \n
> +
> + my $openid_config = {
> + issuer_url => $config->{'issuer-url'},
> + client_id => $config->{'client-id'},
> + client_key => $config->{'client-key'},
> + };
> +
> + my $openid = PVE::RS::OpenId->discover($openid_config, $redirect_url);
this also potentially dies and does not have the newline appended, maybe
wrap in an eval? (there are probably quite a few of those I missed in
the endpoints below)
also, most likely an error here happens when either of the three
parameters is wrong / not matching what the provider expects. when
testing I got 404 here because I entered the full issuer-url instead of
just the https://FQDN part, maybe we could improve the error handling
story in proxmox-openid-connect-rs by being more explicit which steps
fail, or even detecting common pitfalls like that..
> + return ($config, $openid);
> +};
> +
> +__PACKAGE__->register_method ({
> + name => 'index',
> + path => '',
> + method => 'GET',
> + description => "Directory index.",
> + permissions => {
> + user => 'all',
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {},
> + },
> + returns => {
> + type => 'array',
> + items => {
> + type => "object",
> + properties => {
> + subdir => { type => 'string' },
> + },
> + },
> + links => [ { rel => 'child', href => "{subdir}" } ],
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + return [
> + { subdir => 'auth-url' },
> + { subdir => 'login' },
> + ];
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'auth_url',
> + path => 'auth-url',
> + method => 'POST',
> + protected => 1,
> + description => "Get the OpenId Authorization Url for the specified realm.",
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + realm => get_standard_option('realm'),
> + 'redirect-url' => {
> + description => "Redirection Url. The client should set this to the used server url (location.origin).",
> + type => 'string',
> + maxLength => 255,
> + },
> + },
> + },
> + returns => {
> + type => "string",
> + description => "Redirection URL.",
> + },
> + permissions => { user => 'world' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $realm = extract_param($param, 'realm');
> + my $redirect_url = extract_param($param, 'redirect-url');
> +
> + my ($config, $openid) = $lookup_openid_auth->($realm, $redirect_url);
> + my $url = $openid->authorize_url($openid_state_path , $realm);
> +
> + return $url;
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'login',
> + path => 'login',
> + method => 'POST',
> + protected => 1,
> + description => " Verify OpenID authorization code and create a ticket.",
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + 'state' => {
> + description => "OpenId state.",
> + type => 'string',
> + maxLength => 1024,
> + },
> + code => {
> + description => "OpenId authorization code.",
> + type => 'string',
> + maxLength => 1024,
> + },
> + 'redirect-url' => {
> + description => "Redirection Url. The client should set this to the used server url (location.origin).",
> + type => 'string',
> + maxLength => 255,
> + },
> + },
> + },
> + returns => {
> + properties => {
> + username => { type => 'string' },
> + ticket => { type => 'string' },
> + CSRFPreventionToken => { type => 'string' },
> + cap => { type => 'object' }, # computed api permissions
> + clustername => { type => 'string', optional => 1 },
> + },
> + },
> + permissions => { user => 'world' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $rpcenv = PVE::RPCEnvironment::get();
> +
> + my $res;
> + eval {
> + use Data::Dumper;
> +
> + # fixme /tmp
leftover fixme? this is now in /var/lib/pve-manager AFAICT..
> + my ($realm, $private_auth_state) = PVE::RS::OpenId::verify_public_auth_state(
> + $openid_state_path, $param->{'state'});
> +
> + my $redirect_url = extract_param($param, 'redirect-url');
> +
> + my ($config, $openid) = $lookup_openid_auth->($realm, $redirect_url);
> +
> + my $info = $openid->verify_authorization_code($param->{code}, $private_auth_state);
> + my $subject = $info->{'sub'};
> +
> + die "missing openid claim 'sub'" if !defined($subject);
missing \n
> +
> + my $unique_name = $subject; # default
> + if (defined(my $user_attr = $config->{'user-attr'})) {
> + if ($user_attr eq 'subject') {
> + $unique_name = $subject;
> + } elsif ($user_attr eq 'username') {
> + my $username = $info->{'preferred_username'};
> + die "missing claim 'preferred_username'" if !defined($username);
same
> + $unique_name = $username;
> + } elsif ($user_attr eq 'email') {
> + my $email = $info->{'email'};
> + die "missing claim 'email'" if !defined($email);
same
> + $unique_name = $email;
> + } else {
> + die "got unexpected value for user_attr '${user_attr}'";
same
> + }
> + }
> +
> + my $username = "${unique_name}\@${realm}";
> +
> + # test if user exists and is enabled
> + $rpcenv->check_user_enabled($username);
missing check for user being expired
> +
> + my $ticket = PVE::AccessControl::assemble_ticket($username);
> + my $csrftoken = PVE::AccessControl::assemble_csrf_prevention_token($username);
> + my $cap = $rpcenv->compute_api_permission($username);
> +
> + $res = {
> + ticket => $ticket,
> + username => $username,
> + CSRFPreventionToken => $csrftoken,
> + cap => $cap,
> + };
> +
> + my $clinfo = PVE::Cluster::get_clinfo();
> + if ($clinfo->{cluster}->{name} && $rpcenv->check($username, '/', ['Sys.Audit'], 1)) {
> + $res->{clustername} = $clinfo->{cluster}->{name};
> + }
> + };
> + if (my $err = $@) {
> + my $clientip = $rpcenv->get_client_ip() || '';
> + syslog('err', "openid authentication failure; rhost=$clientip msg=$err");
> + # do not return any info to prevent user enumeration attacks
> + die PVE::Exception->new("authentication failure\n", code => 401);
> + }
> +
> + PVE::Cluster::log_msg('info', 'root@pam', "successful openid auth for user '$res->{username}'");
> +
> + return $res;
> + }});
> diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm
> index 2e5a33b..8aae094 100644
> --- a/src/PVE/RPCEnvironment.pm
> +++ b/src/PVE/RPCEnvironment.pm
> @@ -126,6 +126,55 @@ sub permissions {
> return &$compile_acl_path($self, $user, $path);
> }
>
> +sub compute_api_permission {
> + my ($self, $authuser) = @_;
> +
> + my $usercfg = $self->{user_cfg};
> +
> + my $res = {};
> + my $priv_re_map = {
> + vms => qr/VM\.|Permissions\.Modify/,
> + access => qr/(User|Group)\.|Permissions\.Modify/,
> + storage => qr/Datastore\.|Permissions\.Modify/,
> + nodes => qr/Sys\.|Permissions\.Modify/,
> + sdn => qr/SDN\.|Permissions\.Modify/,
> + dc => qr/Sys\.Audit|SDN\./,
> + };
> + map { $res->{$_} = {} } keys %$priv_re_map;
> +
> + my $required_paths = ['/', '/nodes', '/access/groups', '/vms', '/storage', '/sdn'];
> +
> + my $checked_paths = {};
> + foreach my $path (@$required_paths, keys %{$usercfg->{acl}}) {
> + next if $checked_paths->{$path};
> + $checked_paths->{$path} = 1;
> +
> + my $path_perm = $self->permissions($authuser, $path);
> +
> + my $toplevel = ($path =~ /^\/(\w+)/) ? $1 : 'dc';
> + if ($toplevel eq 'pool') {
> + foreach my $priv (keys %$path_perm) {
> + if ($priv =~ m/^VM\./) {
> + $res->{vms}->{$priv} = 1;
> + } elsif ($priv =~ m/^Datastore\./) {
> + $res->{storage}->{$priv} = 1;
> + } elsif ($priv eq 'Permissions.Modify') {
> + $res->{storage}->{$priv} = 1;
> + $res->{vms}->{$priv} = 1;
> + }
> + }
> + } else {
> + my $priv_regex = $priv_re_map->{$toplevel} // next;
> + foreach my $priv (keys %$path_perm) {
> + next if $priv !~ m/^($priv_regex)/;
> + $res->{$toplevel}->{$priv} = 1;
> + }
> + }
> + }
> +
> + return $res;
> +}
> +
> sub get_effective_permissions {
> my ($self, $user) = @_;
>
> --
> 2.30.2
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pve-devel] [PATCH pve-access-control 4/4] implement OpenID autocreate user feature
2021-06-24 8:17 [pve-devel] [PATCH pve-access-control 1/4] add OpenId configuration Dietmar Maurer
` (2 preceding siblings ...)
2021-06-24 8:18 ` [pve-devel] [PATCH pve-access-control 3/4] api: implement openid API Dietmar Maurer
@ 2021-06-24 8:18 ` Dietmar Maurer
2021-06-29 8:29 ` [pve-devel] [PATCH pve-access-control 1/4] add OpenId configuration Fabian Grünbichler
4 siblings, 0 replies; 11+ messages in thread
From: Dietmar Maurer @ 2021-06-24 8:18 UTC (permalink / raw)
To: pve-devel
---
src/PVE/API2/OpenId.pm | 33 ++++++++++++++++++++++++++++++---
1 file changed, 30 insertions(+), 3 deletions(-)
diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
index db9f9eb..3814895 100644
--- a/src/PVE/API2/OpenId.pm
+++ b/src/PVE/API2/OpenId.pm
@@ -9,9 +9,10 @@ use PVE::RS::OpenId;
use PVE::Exception qw(raise raise_perm_exc raise_param_exc);
use PVE::SafeSyslog;
use PVE::RPCEnvironment;
-use PVE::Cluster qw(cfs_read_file);
+use PVE::Cluster qw(cfs_read_file cfs_write_file);
use PVE::AccessControl;
use PVE::JSONSchema qw(get_standard_option);
+use PVE::Auth::Plugin;
use PVE::RESTHandler;
@@ -182,8 +183,34 @@ __PACKAGE__->register_method ({
my $username = "${unique_name}\@${realm}";
- # test if user exists and is enabled
- $rpcenv->check_user_enabled($username);
+ # first, check if $username respects our naming conventions
+ PVE::Auth::Plugin::verify_username($username);
+
+ if ($config->{'autocreate'} && !$rpcenv->check_user_exist($username, 1)) {
+ PVE::AccessControl::lock_user_config(sub {
+ my $usercfg = cfs_read_file("user.cfg");
+
+ die "user '$username' already exists\n" if $usercfg->{users}->{$username};
+
+ my $entry = { enable => 1 };
+ if (defined(my $email = $info->{'email'})) {
+ $entry->{email} = $email;
+ }
+ if (defined(my $given_name = $info->{'given_name'})) {
+ $entry->{firstname} = $given_name;
+ }
+ if (defined(my $family_name = $info->{'family_name'})) {
+ $entry->{lastname} = $family_name;
+ }
+
+ $usercfg->{users}->{$username} = $entry;
+
+ cfs_write_file("user.cfg", $usercfg);
+ }, "autocreate openid user failed");
+ } else {
+ # test if user exists and is enabled
+ $rpcenv->check_user_enabled($username);
+ }
my $ticket = PVE::AccessControl::assemble_ticket($username);
my $csrftoken = PVE::AccessControl::assemble_csrf_prevention_token($username);
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [pve-devel] [PATCH pve-access-control 1/4] add OpenId configuration
2021-06-24 8:17 [pve-devel] [PATCH pve-access-control 1/4] add OpenId configuration Dietmar Maurer
` (3 preceding siblings ...)
2021-06-24 8:18 ` [pve-devel] [PATCH pve-access-control 4/4] implement OpenID autocreate user feature Dietmar Maurer
@ 2021-06-29 8:29 ` Fabian Grünbichler
4 siblings, 0 replies; 11+ messages in thread
From: Fabian Grünbichler @ 2021-06-29 8:29 UTC (permalink / raw)
To: Proxmox VE development discussion
On June 24, 2021 10:17 am, Dietmar Maurer wrote:
> ---
> src/PVE/AccessControl.pm | 2 ++
> src/PVE/Auth/Makefile | 3 +-
> src/PVE/Auth/OpenId.pm | 67 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 71 insertions(+), 1 deletion(-)
> create mode 100755 src/PVE/Auth/OpenId.pm
>
> diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
> index 2569a35..8efb89d 100644
> --- a/src/PVE/AccessControl.pm
> +++ b/src/PVE/AccessControl.pm
> @@ -24,6 +24,7 @@ use PVE::Auth::AD;
> use PVE::Auth::LDAP;
> use PVE::Auth::PVE;
> use PVE::Auth::PAM;
> +use PVE::Auth::OpenId;
>
> # load and initialize all plugins
>
> @@ -31,6 +32,7 @@ PVE::Auth::AD->register();
> PVE::Auth::LDAP->register();
> PVE::Auth::PVE->register();
> PVE::Auth::PAM->register();
> +PVE::Auth::OpenId->register();
> PVE::Auth::Plugin->init();
>
> # $authdir must be writable by root only!
> diff --git a/src/PVE/Auth/Makefile b/src/PVE/Auth/Makefile
> index 58ae362..be7bde3 100644
> --- a/src/PVE/Auth/Makefile
> +++ b/src/PVE/Auth/Makefile
> @@ -4,7 +4,8 @@ AUTH_SOURCES= \
> PVE.pm \
> PAM.pm \
> AD.pm \
> - LDAP.pm
> + LDAP.pm \
> + OpenId.pm
>
> .PHONY: install
> install:
> diff --git a/src/PVE/Auth/OpenId.pm b/src/PVE/Auth/OpenId.pm
> new file mode 100755
> index 0000000..8f35575
> --- /dev/null
> +++ b/src/PVE/Auth/OpenId.pm
> @@ -0,0 +1,67 @@
> +package PVE::Auth::OpenId;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Tools;
> +use PVE::Auth::Plugin;
> +use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file cfs_lock_file);
> +
> +use base qw(PVE::Auth::Plugin);
> +
> +sub type {
> + return 'openid';
> +}
> +
> +sub properties {
> + return {
> + "issuer-url" => {
> + description => "OpenID Issuer Url",
> + type => 'string',
> + maxLength => 256,
> + },
> + "client-id" => {
> + description => "OpenID Client ID",
> + type => 'string',
> + maxLength => 256,
> + },
> + "client-key" => {
> + description => "OpenID Client Key",
> + type => 'string',
> + optional => 1,
> + maxLength => 256,
> + },
> + autocreate => {
> + description => "Automatically create users if they do not exist.",
> + optional => 1,
> + type => 'boolean',
> + default => 0,
> + },
> + "user-attr" => {
> + type => 'string',
> + enum => ['subject', 'username', 'email'],
> + optional => 1,
> + },
clashes with existing 'user_attr' for LDAP..
> + };
> +}
> +
> +sub options {
> + return {
> + "issuer-url" => {},
> + "client-id" => {},
> + "client-key" => { optional => 1 },
> + autocreate => { optional => 1 },
> + "user-attr" => { optional => 1, fixed => 1 },
> + default => { optional => 1 },
> + comment => { optional => 1 },
> + };
> +}
> +
> +sub authenticate_user {
> + my ($class, $config, $realm, $username, $password) = @_;
> +
> + die "OpenID realm does not allow password verification.\n";
> +}
> +
> +
> +1;
> --
> 2.30.2
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread