Skip to content
This repository has been archived by the owner on Apr 28, 2020. It is now read-only.

Commit

Permalink
Change Client.key to a UUID
Browse files Browse the repository at this point in the history
  • Loading branch information
jace committed Mar 20, 2020
1 parent 4a907cc commit 4cbde6d
Show file tree
Hide file tree
Showing 9 changed files with 131 additions and 38 deletions.
16 changes: 7 additions & 9 deletions lastuser_core/models/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from baseframe import _
from coaster.utils import buid, newsecret, require_one_of, utcnow

from . import BaseMixin, db
from . import BaseMixin, UuidMixin, db
from .session import UserSession
from .user import Organization, Team, User

Expand Down Expand Up @@ -57,7 +57,7 @@ def add_scope(self, additional):
self.scope = list(set(self.scope).union(set(additional)))


class Client(ScopeMixin, BaseMixin, db.Model):
class Client(ScopeMixin, UuidMixin, BaseMixin, db.Model):
"""OAuth client applications"""

__tablename__ = 'client'
Expand Down Expand Up @@ -96,8 +96,6 @@ class Client(ScopeMixin, BaseMixin, db.Model):
active = db.Column(db.Boolean, nullable=False, default=True)
#: Allow anyone to login to this app?
allow_any_login = db.Column(db.Boolean, nullable=False, default=True)
#: OAuth client key/id
key = db.Column(db.String(22), nullable=False, unique=True, default=buid)
#: Trusted flag: trusted clients are authorized to access user data
#: without user consent, but the user must still login and identify themself.
#: When a single provider provides multiple services, each can be declared
Expand All @@ -123,7 +121,7 @@ class Client(ScopeMixin, BaseMixin, db.Model):
)

def __repr__(self):
return '<Client "{title}" {key}>'.format(title=self.title, key=self.key)
return '<Client "{title}" {buid}>'.format(title=self.title, buid=self.buid)

def secret_is(self, candidate, name):
"""
Expand Down Expand Up @@ -186,14 +184,14 @@ def authtoken_for(self, user, user_session=None):
).one_or_none()

@classmethod
def get(cls, key=None, namespace=None):
def get(cls, buid=None, namespace=None):
"""
Return a Client identified by its client key or namespace. Only returns active clients.
Return a Client identified by its client buid or namespace. Only returns active clients.
:param str key: Client key to lookup
:param str buid: Client buid to lookup
:param str namespace: Client namespace to lookup
"""
param, value = require_one_of(True, key=key, namespace=namespace)
param, value = require_one_of(True, buid=buid, namespace=namespace)
return cls.query.filter_by(**{param: value, 'active': True}).one_or_none()


Expand Down
2 changes: 1 addition & 1 deletion lastuser_oauth/templates/login_beacon.html.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
xhr.send();
};
getJSON({{ url_for('.login_beacon_json', client_id=client.key)|tojson }}, function(data) {
getJSON({{ url_for('.login_beacon_json', client_id=client.buid)|tojson }}, function(data) {
if (data.hastoken) {
window.top.location.href = {{ login_url|tojson }} + '?next=' + encodeURIComponent(document.referrer);
} // Else: no user, don't do anything
Expand Down
4 changes: 2 additions & 2 deletions lastuser_oauth/views/resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ def token_verify():
'uuid': authtoken.client.owner.uuid,
'owner_title': authtoken.client.owner.pickername,
'website': authtoken.client.website,
'key': authtoken.client.key,
'key': authtoken.client.buid,
'trusted': authtoken.client.trusted,
}
return api_result('ok', **params)
Expand Down Expand Up @@ -289,7 +289,7 @@ def token_get_scope():
'uuid': authtoken.client.owner.uuid,
'owner_title': authtoken.client.owner.pickername,
'website': authtoken.client.website,
'key': authtoken.client.key,
'key': authtoken.client.buid,
'trusted': authtoken.client.trusted,
'scope': client_resources,
}
Expand Down
2 changes: 1 addition & 1 deletion lastuser_ui/templates/client_cred.html.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@
</dl>
</div>
<p>
<a class="mui-btn mui-btn--small mui-btn--raised mui-btn--primary" href="{{ url_for('.client_info', key=cred.client.key) }}"><i class="fa fa-angle-left"></i> Return to client info</a>
<a class="mui-btn mui-btn--small mui-btn--raised mui-btn--primary" href="{{ url_for('.client_info', key=cred.client.buid) }}"><i class="fa fa-angle-left"></i> Return to client info</a>
</p>
{% endblock %}
14 changes: 7 additions & 7 deletions lastuser_ui/templates/client_info.html.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
</div>
<div class="card__body card--text mui--text-subhead">
<ol class="mui-list--aligned">
<li><a href="{{ url_for('.client_edit', key=client.key) }}" title="Edit">Edit this application</a></li>
<li><a href="{{ url_for('.client_delete', key=client.key) }}" title="Delete">Delete</a></li>
<li><a href="{{ url_for('.client_cred_new', key=client.key) }}" title="New access key">New access key</a></li>
<li><a href="{{ url_for('.client_edit', key=client.buid) }}" title="Edit">Edit this application</a></li>
<li><a href="{{ url_for('.client_delete', key=client.buid) }}" title="Delete">Delete</a></li>
<li><a href="{{ url_for('.client_cred_new', key=client.buid) }}" title="New access key">New access key</a></li>
<li><a href="{{ url_for('.permission_new') }}">Define a new permission</a></li>
<li><a href="{{ url_for('.permission_user_new', key=client.key) }}">Assign permissions to another {% if client.user %}user{% else %}team{% endif %}</a></li>
<li><a href="{{ url_for('.permission_user_new', key=client.buid) }}">Assign permissions to another {% if client.user %}user{% else %}team{% endif %}</a></li>
</ol>
</div>
</div>
Expand Down Expand Up @@ -83,7 +83,7 @@
</div>
<div class="mui-divider"></div>
<div class="card__footer">
<a href="{{ url_for('.client_cred_delete', key=client.key, name=cred.name) }}" title="{% trans %}Revoke{% endtrans %}" class="mui-btn mui-btn--small mui-btn--flat mui-btn--primary">Revoke</a>
<a href="{{ url_for('.client_cred_delete', key=client.buid, name=cred.name) }}" title="{% trans %}Revoke{% endtrans %}" class="mui-btn mui-btn--small mui-btn--flat mui-btn--primary">Revoke</a>
</div>
</div>
</div>
Expand Down Expand Up @@ -112,8 +112,8 @@
</div>
<div class="mui-divider"></div>
<div class="card__footer">
<a href="{{ url_for('.permission_user_edit', key=client.key, buid=pa.buid) }}" title="{% trans %}Edit{% endtrans %}" class="mui-btn mui-btn--small mui-btn--flat mui-btn--primary">Edit</a>
<a href="{{ url_for('.permission_user_delete', key=client.key, buid=pa.buid) }}" title="{% trans %}Delete{% endtrans %}" class="mui-btn mui-btn--small mui-btn--flat mui-btn--primary">Delete</a>
<a href="{{ url_for('.permission_user_edit', key=client.buid, buid=pa.buid) }}" title="{% trans %}Edit{% endtrans %}" class="mui-btn mui-btn--small mui-btn--flat mui-btn--primary">Edit</a>
<a href="{{ url_for('.permission_user_delete', key=client.buid, buid=pa.buid) }}" title="{% trans %}Delete{% endtrans %}" class="mui-btn mui-btn--small mui-btn--flat mui-btn--primary">Delete</a>
</div>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion lastuser_ui/templates/client_list.html.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
</thead>
<tbody class="mui--text-subhead">
{% for client in clients %}
{%- set link = url_for('.client_info', key=client.key)|e %}
{%- set link = url_for('.client_info', key=client.buid)|e %}
<tr>
<td data-th="#"><a href="{{ link }}" title="{{ client.title }}">{{ loop.index }}</a></td>
<td data-th="Title"><a href="{{ link }}" title="{{ client.title }}">{{ client.title }}</a></td>
Expand Down
30 changes: 15 additions & 15 deletions lastuser_ui/views/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def client_new():
client.trusted = False
db.session.add(client)
db.session.commit()
return render_redirect(url_for('.client_info', key=client.key), code=303)
return render_redirect(url_for('.client_info', buid=client.buid), code=303)

return render_form(
form=form,
Expand All @@ -98,7 +98,7 @@ def client_new():


@lastuser_ui.route('/apps/<key>')
@load_model(Client, {'key': 'key'}, 'client', permission='view')
@load_model(Client, {'buid': 'key'}, 'client', permission='view')
def client_info(client):
if client.user:
permassignments = UserClientPermissions.query.filter_by(client=client).all()
Expand All @@ -111,7 +111,7 @@ def client_info(client):

@lastuser_ui.route('/apps/<key>/edit', methods=['GET', 'POST'])
@requires_login
@load_model(Client, {'key': 'key'}, 'client', permission='edit')
@load_model(Client, {'buid': 'key'}, 'client', permission='edit')
def client_edit(client):
form = RegisterClientForm(obj=client, model=Client)
form.edit_user = current_auth.user
Expand Down Expand Up @@ -140,7 +140,7 @@ def client_edit(client):
client.user = form.user
client.org = form.org
db.session.commit()
return render_redirect(url_for('.client_info', key=client.key), code=303)
return render_redirect(url_for('.client_info', key=client.buid), code=303)

return render_form(
form=form,
Expand All @@ -153,7 +153,7 @@ def client_edit(client):

@lastuser_ui.route('/apps/<key>/delete', methods=['GET', 'POST'])
@requires_login
@load_model(Client, {'key': 'key'}, 'client', permission='delete')
@load_model(Client, {'buid': 'key'}, 'client', permission='delete')
def client_delete(client):
return render_delete_sqla(
client,
Expand All @@ -172,7 +172,7 @@ def client_delete(client):

@lastuser_ui.route('/apps/<key>/cred', methods=['GET', 'POST'])
@requires_login
@load_model(Client, {'key': 'key'}, 'client', permission='edit')
@load_model(Client, {'buid': 'key'}, 'client', permission='edit')
def client_cred_new(client):
form = ClientCredentialForm()
if request.method == 'GET' and not client.credentials:
Expand All @@ -196,7 +196,7 @@ def client_cred_new(client):
@lastuser_ui.route('/apps/<key>/cred/<name>/delete', methods=['GET', 'POST'])
@requires_login
@load_models(
(Client, {'key': 'key'}, 'client'),
(Client, {'buid': 'key'}, 'client'),
(ClientCredential, {'name': 'name', 'client': 'client'}, 'cred'),
permission='delete',
)
Expand All @@ -207,7 +207,7 @@ def client_cred_delete(client, cred):
title=_("Confirm delete"),
message=_("Delete access key ‘{title}’? ").format(title=cred.title),
success=_("You have deleted access key ‘{title}’").format(title=cred.title),
next=url_for('.client_info', key=client.key),
next=url_for('.client_info', key=client.buid),
)


Expand Down Expand Up @@ -307,7 +307,7 @@ def permission_delete(perm):

@lastuser_ui.route('/apps/<key>/perms/new', methods=['GET', 'POST'])
@requires_login
@load_model(Client, {'key': 'key'}, 'client', permission='assign-permissions')
@load_model(Client, {'buid': 'key'}, 'client', permission='assign-permissions')
def permission_user_new(client):
if client.user:
available_perms = (
Expand Down Expand Up @@ -374,7 +374,7 @@ def permission_user_new(client):
),
'success',
)
return render_redirect(url_for('.client_info', key=client.key), code=303)
return render_redirect(url_for('.client_info', key=client.buid), code=303)
return render_form(
form=form,
title=_("Assign permissions"),
Expand All @@ -386,7 +386,7 @@ def permission_user_new(client):
@lastuser_ui.route('/apps/<key>/perms/<buid>/edit', methods=['GET', 'POST'])
@requires_login
@load_model(
Client, {'key': 'key'}, 'client', permission='assign-permissions', kwargs=True
Client, {'buid': 'key'}, 'client', permission='assign-permissions', kwargs=True
)
def permission_user_edit(client, kwargs):
if client.user:
Expand Down Expand Up @@ -465,7 +465,7 @@ def permission_user_edit(client, kwargs):
),
'success',
)
return render_redirect(url_for('.client_info', key=client.key), code=303)
return render_redirect(url_for('.client_info', key=client.buid), code=303)
return render_form(
form=form,
title=_("Edit permissions"),
Expand All @@ -478,7 +478,7 @@ def permission_user_edit(client, kwargs):
@lastuser_ui.route('/apps/<key>/perms/<buid>/delete', methods=['GET', 'POST'])
@requires_login
@load_model(
Client, {'key': 'key'}, 'client', permission='assign-permissions', kwargs=True
Client, {'buid': 'key'}, 'client', permission='assign-permissions', kwargs=True
)
def permission_user_delete(client, kwargs):
if client.user:
Expand All @@ -498,7 +498,7 @@ def permission_user_delete(client, kwargs):
success=_("You have revoked permisions for user {pname}").format(
pname=user.pickername
),
next=url_for('.client_info', key=client.key),
next=url_for('.client_info', key=client.buid),
)
else:
team = Team.get(buid=kwargs['buid'])
Expand All @@ -517,5 +517,5 @@ def permission_user_delete(client, kwargs):
success=_("You have revoked permisions for team {title}").format(
title=team.title
),
next=url_for('.client_info', key=client.key),
next=url_for('.client_info', key=client.buid),
)
95 changes: 95 additions & 0 deletions migrations/versions/8a9bf9d385c2_make_client_key_a_uuid.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# -*- coding: utf-8 -*-
"""Make client.key a UUID
Revision ID: 8a9bf9d385c2
Revises: f65c00c0cfc3
Create Date: 2020-03-20 18:43:41.802182
"""
from alembic import op
from sqlalchemy.sql import column, table
from sqlalchemy_utils import UUIDType
import sqlalchemy as sa

from progressbar import ProgressBar
import progressbar.widgets

from coaster.utils import buid2uuid, uuid2buid

# revision identifiers, used by Alembic.
revision = '8a9bf9d385c2'
down_revision = 'f65c00c0cfc3'
branch_labels = None
depends_on = None


client = table(
'client',
column('id', sa.Integer()),
column('key', sa.Unicode(22)),
column('uuid', UUIDType(binary=False)),
)


def get_progressbar(label, maxval):
return ProgressBar(
maxval=maxval,
widgets=[
label,
': ',
progressbar.widgets.Percentage(),
' ',
progressbar.widgets.Bar(),
' ',
progressbar.widgets.ETA(),
' ',
],
)


def upgrade():
op.add_column('client', sa.Column('uuid', UUIDType(binary=False), nullable=True))

conn = op.get_bind()
count = conn.scalar(sa.select([sa.func.count('*')]).select_from(client))
progress = get_progressbar("Clients", count)
progress.start()
items = conn.execute(sa.select([client.c.id, client.c.key]))
for counter, item in enumerate(items):
conn.execute(
sa.update(client)
.where(client.c.id == item.id)
.values(uuid=buid2uuid(item.key))
)
progress.update(counter)
progress.finish()

op.alter_column('client', 'uuid', nullable=False)
op.drop_constraint('client_key_key', 'client', type_='unique')
op.create_unique_constraint('client_uuid_key', 'client', ['uuid'])
op.drop_column('client', 'key')


def downgrade():
op.add_column(
'client',
sa.Column('key', sa.VARCHAR(length=22), autoincrement=False, nullable=False),
)

conn = op.get_bind()
count = conn.scalar(sa.select([sa.func.count('*')]).select_from(client))
progress = get_progressbar("Clients", count)
progress.start()
items = conn.execute(sa.select([client.c.id, client.c.uuid]))
for counter, item in enumerate(items):
conn.execute(
sa.update(client)
.where(client.c.id == item.id)
.values(key=uuid2buid(item.uuid))
)
progress.update(counter)
progress.finish()

op.drop_constraint('client_uuid_key', 'client', type_='unique')
op.create_unique_constraint('client_key_key', 'client', ['key'])
op.drop_column('client', 'uuid')
4 changes: 2 additions & 2 deletions tests/unit/lastuser_core/test_model_client_Client.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,14 @@ def test_client_get(self):
"""
client = self.fixtures.client
batdog = self.fixtures.batdog
key = client.key
key = client.buid
# scenario 1: when no key or namespace
with self.assertRaises(TypeError):
models.Client.get()
# scenario 2: when given key
result1 = models.Client.get(key)
self.assertIsInstance(result1, models.Client)
self.assertEqual(result1.key, key)
self.assertEqual(result1.buid, key)
self.assertEqual(result1.owner, batdog)
# scenario 3: when given namespace
namespace = 'fun.batdogadventures.com'
Expand Down

0 comments on commit 4cbde6d

Please sign in to comment.