Skip to content

Commit

Permalink
Add template and model view changes for organization authorization se…
Browse files Browse the repository at this point in the history
…ttings (#277)

* Add template and model view changes for organization authorization settings

This relies on readthedocs/readthedocs-corporate#1730

This drops the very custom form for a more native form, but retains some
of the styles of the authorization provider list. This is now just a
informational block describing the providers.

This also adds the KO view logic back to the view, so that we can
conditionally show/require the domain field.

Also raised in the PR above, a warning block is added when the provider
value is changing, so there is some information that
authentication/authorization might fail.

* More changes to support additional validation on domain field

* Update orb

* Copy changes

* Review changes on copy

Co-authored-by: Manuel Kaufmann <[email protected]>

---------

Co-authored-by: Manuel Kaufmann <[email protected]>
  • Loading branch information
agjohnson and humitos authored Feb 27, 2024
1 parent 480e374 commit e063afa
Show file tree
Hide file tree
Showing 4 changed files with 182 additions and 95 deletions.

Large diffs are not rendered by default.

170 changes: 76 additions & 94 deletions readthedocsext/theme/templates/organizations/settings/sso_edit.html
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
{% extends "organizations/settings/base.html" %}

{% load i18n %}
{% load organizations %}
{% load corporate %}
{% load crispy_forms_tags %}

{% load corporate %}
{% load ext_theme_tags %}
{% load organizations %}

{% block title %}{{ organization.name }} - {% trans "Authorization" %}{% endblock %}

Expand Down Expand Up @@ -41,104 +42,85 @@
</div>
{% endif %}

<form class="ui form" method="POST" action=".">
{% csrf_token %}

{% comment %}
Django built in forms don't provide enough flexibility to make this UI nice,
so we build the entire form outside the backend forms and models instead.

{# form | crispy #}
{% endcomment %}


<div class="grouped fields">

{# TODO Reused the existing title for now, but is "authorization" the most correct here? #}
<label for="provider">
{% trans "Organization authorization provider:" %}
</label>
<div class="field">
<div class="ui radio checkbox checked" data-bind="semanticui: {checkbox: {}}">
<label>
<strong>{% trans "Read the Docs teams" %}</strong>
<p>
{% blocktrans trimmed %}
Use built-in organization teams to manage access to your projects and documentation.
{% endblocktrans %}
</p>
</label>
<input type="radio" name="provider" tabindex="0" class="hidden" value checked />
<div data-bind="using: new OrganizationSettingsAuthorizationView()">
<form class="ui form" method="POST" action="." data-bind="css: { warning: show_warning }">
{% csrf_token %}

<div class="ui top attached segment">
<div class="ui relaxed list">

<div class="item">
<i class="fad fa-user-group medium middle aligned icon"></i>
<div class="content">
<div class="header">
{{ form.PROVIDER_DEFAULT }}
<span class="ui horizontal label">
{% trans "Default" %}
</span>

</div>
<div class="description">
{% blocktrans trimmed %}
Use built-in organization teams to manage access to your projects and documentation.
{% endblocktrans %}
</div>
</div>
</div>

<div class="item">
<i class="fab fa-github medium middle aligned icon"></i>
<div class="content">
<div class="header">{{ form.PROVIDER_ALLAUTH }}</div>
<div class="description">
{% blocktrans trimmed %}
Control all access to your projects by reusing permissions from your repository's version control provider.
This replaces Read the Docs managed teams.
{% endblocktrans %}
</div>
</div>
</div>

<div class="item">
<i class="fab fa-google middle aligned icon"></i>
<div class="content">
<div class="header">
{{ form.PROVIDER_EMAIL }}
<a class="ui secondary horizontal label" href="{% url 'subscription_detail' organization.slug %}">
Pro plan
</a>
</div>
<div class="description">
{% blocktrans trimmed %}
Users with a verified Google Workspace email account matching your domain will auto-join your organization.
All users in your organization are required to have a connected Google account.
Project permissions and authorization are managed using Read the Docs teams.
{% endblocktrans %}
</div>
</div>
</div>

</div>
</div>

<div class="field">
<div class="ui radio checkbox" data-bind="semanticui: {checkbox: {}}">
<label>
<strong>{% trans "GitHub, GitLab, or Bitbucket" %}</strong>
<p>
{% blocktrans trimmed %}
Control all access to your projects by reusing permissions from your project's version control provider.
This replaces Read the Docs managed teams.
{% endblocktrans %}
</p>
</label>
<input type="radio" name="provider" tabindex="0" class="hidden" value="allauth" />
<div class="ui basic fieldset segment">
{% alter_field form.provider data_bind="valueInit: provider, value: provider" %}
{{ form.provider | as_crispy_field }}

<div class="ko hidden" data-bind="css: { hidden: !use_domain() }">
{{ form.domain | as_crispy_field }}
</div>
</div>

<div class="field">
<div class="ui radio disabled checkbox" data-bind="semanticui: {checkbox: {}}">
<label>
<strong>Google</strong>
{% if not has_google_sso_feature %}
<span class="ui small secondary label">
Pro plan
</span>
{% endif %}
<p>
{% blocktrans trimmed %}
Include all users from your Google Domains organization in a specific team,
allowing them access to a subset of your projects.
{% endblocktrans %}
</p>
</label>
<input type="radio" name="provider" tabindex="0" class="hidden" value="email" />
<div class="ui warning message ko hidden" data-bind="css: { hidden: !show_warning() }">
<i class="fad fa-circle-exclamation icon"></i>
{% blocktrans trimmed %}
When switching authentication providers, user permissions granted
through your current provider may be discarded. Ensure your new
authentication provider is configured correctly before making this change.
{% endblocktrans %}
</div>
{% if not has_google_sso_feature %}
{# Because the field above is disabled, the label above isn't linkable. This is a redundant element for now #}
<a class="ui small basic secondary pointing up label" href="{% url 'subscription_detail' organization.slug %}">
{% trans "Upgrade your plan" %}
</a>
{% endif %}
</div>

</div>

{# TODO expose domain field and move jQuery logic to a knockout view #}
{% comment %}
{% block extra_scripts %}
<script type="text/javascript">
$(document).ready(function() {
var value = $('#id_provider').val();
if (value !== 'email') {
$('#id_domain').parent().hide();
}

$('#id_provider').bind('change', function (ev) {
if (this.value === 'email') {
$('#id_domain').parent().show();
$('#id_domain').attr('required', true);
} else {
$('#id_domain').parent().hide();
$('#id_domain').attr('required', false);
}
})
})
</script>
{% endblock %}
{% endcomment %}

<input class="ui primary button" type="submit" name="submit" value="{% trans "Save" %}" />
</form>
<input class="ui primary button" type="submit" name="submit" value="{% trans "Save" %}" />
</form>
</div>
{% endblock organization_edit_content %}
37 changes: 37 additions & 0 deletions src/js/organization/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,40 @@ export class OrganizationListView {
}
}
Registry.add_view(OrganizationListView);

/**
* Organization authorization settings
*/
export class OrganizationSettingsAuthorizationView {
static view_name = "OrganizationSettingsAuthorizationView";

constructor() {
this.provider_original = undefined;
this.provider = ko.observable();

this.show_warning = ko.observable(false);
this.provider.subscribe(
(value) => {
if (this.provider_original === undefined) {
this.provider_original = value;
}
},
this,
"beforeChange",
);
this.provider.subscribe((value) => {
if (
this.provider_original !== undefined &&
value !== this.provider_original
) {
this.show_warning(true);
} else {
this.show_warning(false);
}
});
this.use_domain = ko.computed(() => {
return this.provider() === "email";
});
}
}
Registry.add_view(OrganizationSettingsAuthorizationView);
68 changes: 68 additions & 0 deletions src/js/tests/views/organization_authorization.test.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<html>
<body>
<div data-bind="using: TestView()">
<form>
<input
type="text"
name="provider"
value="email"
data-bind="valueInit: provider, value: provider"
/>
</form>
<div data-bind="text: done()"></div>
</div>

<script type="module">
import { expect } from "@open-wc/testing";
import { runTests } from "@web/test-runner-mocha";
import { default as ko } from "knockout";
import { default as jquery } from "jquery";

import { Application } from "../../application";
import { Registry } from "../../application/registry";
import { OrganizationSettingsAuthorizationView } from "../../organization/index";

const app = new Application({ debug: true });

let view;
let promise;

class TestView extends OrganizationSettingsAuthorizationView {
static view_name = "TestView";

constructor() {
super();
view = this;
promise = new Promise((resolve) => {
this._promise_resolve = resolve;
});
}

done() {
this._promise_resolve();
}
}

Registry.add_view(TestView);
app.run();

runTests(async () => {
describe("Organization settings authorization view", () => {
it("observables show correct changes", async () => {
await promise;
expect(view.provider_original).to.be.undefined;
expect(view.provider()).to.be.equal("email");
expect(view.show_warning()).to.be.false;
expect(view.use_domain()).to.be.true;

view.provider("allauth");
expect(view.provider_original).to.be.equal("email");
expect(view.provider()).to.be.equal("allauth");
expect(view.show_warning()).to.be.true;
expect(view.use_domain()).to.be.false;
});
});
});
</script>
</body>
</html>

0 comments on commit e063afa

Please sign in to comment.