Repository :
http://git.fedorahosted.org/cgit/copr.git
On branch : master
---------------------------------------------------------------
commit f6d60d0a582fbec78aa1699bebc23112a354e572
Author: Pavel Raiskup <praiskup(a)redhat.com>
Date: Wed Aug 20 22:50:37 2014 +0200
do not strictly resist on Fedora's OpenID
To allow users use multiple authentication options concurrently
(e.g. simply password/hash auth in future & Fedora OpenID), we
need to keep single and unique username in our database. To allow
that, transform the 'openid_name' column from 'user' table into
simple 'username'.
---------------------------------------------------------------
.../bd0a15c7b6f_dont_require_fed_openid.py | 66 ++++++++++++++++++++
.../coprs_frontend/coprs/logic/builds_logic.py | 3 +-
frontend/coprs_frontend/coprs/logic/coprs_logic.py | 8 +--
frontend/coprs_frontend/coprs/models.py | 31 ++++------
frontend/coprs_frontend/coprs/views/misc.py | 21 +++++-
frontend/coprs_frontend/tests/coprs_test_case.py | 12 ++--
.../test_views/test_admin/test_admin_general.py | 4 +-
7 files changed, 108 insertions(+), 37 deletions(-)
diff --git
a/frontend/coprs_frontend/alembic/versions/bd0a15c7b6f_dont_require_fed_openid.py
b/frontend/coprs_frontend/alembic/versions/bd0a15c7b6f_dont_require_fed_openid.py
new file mode 100644
index 0000000..d6e121f
--- /dev/null
+++ b/frontend/coprs_frontend/alembic/versions/bd0a15c7b6f_dont_require_fed_openid.py
@@ -0,0 +1,66 @@
+"""Do not require Fedora's OpenID in DB schema
+
+Revision ID: bd0a15c7b6f
+Revises: 5055336e8c44
+Create Date: 2014-08-20 21:49:05.299819
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'bd0a15c7b6f'
+down_revision = '5055336e8c44'
+
+from alembic import op
+import sqlalchemy as sa
+import logging
+
+logger = logging.getLogger('alembic')
+
+metadata = sa.MetaData()
+
+def username_default(oid_user):
+ return
oid_user.replace(".id.fedoraproject.org/", "") \
+ .replace("http://", "")
+
+def upgrade():
+ op.add_column(u'user', sa.Column('username', sa.String(length=100)))
+
+ sa_user = sa.Table("user", metadata,
+ sa.Column("id", sa.Integer),
+ sa.Column("openid_name", sa.String(length=100)),
+ sa.Column("username", sa.String(length=100))
+ )
+
+ for u in op.get_bind().execute(sa.select([sa_user.c.id, sa_user.c.openid_name])):
+ username = username_default(u[1])
+ logger.info("converting {0}'s account".format(username))
+ op.get_bind().execute(sa_user.update() \
+ .where(sa_user.c.id==u[0]) \
+ .values(username=username))
+
+ if op.get_bind().dialect.name != 'sqlite':
+ # Pretty sad we can not set this non-nullable in SQLite
+ op.alter_column("user", "username", nullable=False)
+ # We can live with redundant openid_name column..
+ op.drop_column(u'user', u'openid_name')
+
+
+def downgrade():
+ op.add_column(u'user', sa.Column(u'openid_name',
sa.VARCHAR(length=100), nullable=True))
+
+ sa_user = sa.Table("user", metadata,
+ sa.Column("id", sa.Integer),
+ sa.Column("openid_name", sa.String(length=100)),
+ sa.Column("username", sa.String(length=100))
+ )
+
+ for u in op.get_bind().execute(sa.select([sa_user.c.id, sa_user.c.username])):
+ openid_name = "http://{0}.id.fedoraproject.org/".format(str(u[1]))
+ op.get_bind().execute(sa_user.update() \
+ .where(sa_user.c.id==u[0]) \
+ .values(openid_name=openid_name)
+ )
+
+ if op.get_bind().dialect.name != 'sqlite':
+ op.drop_column(u'user', 'username')
+ op.alter_column("user", "openid_name", nullable=False)
diff --git a/frontend/coprs_frontend/coprs/logic/builds_logic.py
b/frontend/coprs_frontend/coprs/logic/builds_logic.py
index 38d151f..6f8ffc4 100644
--- a/frontend/coprs_frontend/coprs/logic/builds_logic.py
+++ b/frontend/coprs_frontend/coprs/logic/builds_logic.py
@@ -78,8 +78,7 @@ class BuildsLogic(object):
.options(db.contains_eager(models.Build.copr))
.join(models.Copr.owner)
.filter(models.Copr.name == coprname)
- .filter(models.User.openid_name ==
- models.User.openidize_name(username))
+ .filter(models.User.username == username)
.order_by(models.Build.submitted_on.desc()))
else:
raise exceptions.ArgumentMissingException(
diff --git a/frontend/coprs_frontend/coprs/logic/coprs_logic.py
b/frontend/coprs_frontend/coprs/logic/coprs_logic.py
index 2607ca9..b7ed37b 100644
--- a/frontend/coprs_frontend/coprs/logic/coprs_logic.py
+++ b/frontend/coprs_frontend/coprs/logic/coprs_logic.py
@@ -33,8 +33,7 @@ class CoprsLogic(object):
query = (cls.get_all()
.filter(models.Copr.name == coprname)
- .filter(models.User.openid_name ==
- models.User.openidize_name(username))
+ .filter(models.User.username == username)
)
if with_builds:
@@ -73,7 +72,7 @@ class CoprsLogic(object):
if user_relation == "owned":
query = query.filter(
- models.User.openid_name == models.User.openidize_name(username))
+ models.User.username == username)
elif user_relation == "allowed":
aliased_user = db.aliased(models.User)
@@ -82,8 +81,7 @@ class CoprsLogic(object):
.filter(models.CoprPermission.copr_builder ==
helpers.PermissionEnum('approved'))
.join(aliased_user, models.CoprPermission.user)
- .filter(aliased_user.openid_name ==
- models.User.openidize_name(username)))
+ .filter(aliased_user.username == username))
if with_mock_chroots:
query = (query.outerjoin(*models.Copr.mock_chroots.attr)
diff --git a/frontend/coprs_frontend/coprs/models.py
b/frontend/coprs_frontend/coprs/models.py
index e30b819..420f49c 100644
--- a/frontend/coprs_frontend/coprs/models.py
+++ b/frontend/coprs_frontend/coprs/models.py
@@ -7,25 +7,31 @@ from coprs import constants
from coprs import db
from coprs import helpers
-
class User(db.Model, helpers.Serializer):
"""
Represents user of the copr frontend
"""
+ # PK; TODO: the 'username' could be also PK
id = db.Column(db.Integer, primary_key=True)
- # openid_name for fas, e.g.
http://bkabrda.id.fedoraproject.org/
- openid_name = db.Column(db.String(100), nullable=False)
- # just mail :)
+
+ # unique username
+ username = db.Column(db.String(100), nullable=False, unique=True)
+
+ # email
mail = db.Column(db.String(150), nullable=False)
- # just timezone ;)
+
+ # optional timezone
timezone = db.Column(db.String(50), nullable=True)
+
# is this user proven? proven users can modify builder memory and
# timeout for single builds
proven = db.Column(db.Boolean, default=False)
+
# is this user admin of the system?
admin = db.Column(db.Boolean, default=False)
+
# stuff for the cli interface
api_login = db.Column(db.String(40), nullable=False, default="abc")
api_token = db.Column(db.String(40), nullable=False, default="abc")
@@ -38,8 +44,7 @@ class User(db.Model, helpers.Serializer):
Return the short username of the user, e.g. bkabrda
"""
- return self.openid_name.replace(
- ".id.fedoraproject.org/",
"").replace("http://", "")
+ return self.username
def permissions_for_copr(self, copr):
"""
@@ -87,17 +92,6 @@ class User(db.Model, helpers.Serializer):
return can_edit
- @classmethod
- def openidize_name(cls, name):
- """
- Create proper openid_name from short name.
-
- >>> user.openid_name == User.openidize_name(user.name)
- True
- """
-
- return "http://{0}.id.fedoraproject.org/".format(name)
-
@property
def serializable_attributes(self):
# enumerate here to prevent exposing credentials
@@ -124,7 +118,6 @@ class User(db.Model, helpers.Serializer):
except IOError:
return ""
-
class Copr(db.Model, helpers.Serializer):
"""
diff --git a/frontend/coprs_frontend/coprs/views/misc.py
b/frontend/coprs_frontend/coprs/views/misc.py
index c557511..0d84df7 100644
--- a/frontend/coprs_frontend/coprs/views/misc.py
+++ b/frontend/coprs_frontend/coprs/views/misc.py
@@ -10,13 +10,27 @@ from coprs import helpers
from coprs import models
from coprs import oid
+def fed_openidize_name(name):
+ """
+ Create proper Fedora OpenID name from short name.
+
+ >>> fedoraoid == fed_openidize_name(user.name)
+ True
+ """
+
+ return "http://{0}.id.fedoraproject.org/".format(name)
+
+def fed_raw_name(oidname):
+ return
oidname.replace(".id.fedoraproject.org/", "") \
+ .replace("http://", "")
@app.before_request
def lookup_current_user():
flask.g.user = None
if "openid" in flask.session:
+ username = fed_raw_name(flask.session["openid"])
flask.g.user = models.User.query.filter(
- models.User.openid_name == flask.session["openid"]).first()
+ models.User.username == username).first()
@app.errorhandler(404)
@@ -48,15 +62,16 @@ def create_or_login(resp):
and fasusername in app.config["ALLOWED_USERS"])
or not app.config["USE_ALLOWED_USERS"]):
+ username = fed_raw_name(resp.identity_url)
user = models.User.query.filter(
- models.User.openid_name == resp.identity_url).first()
+ models.User.username == username).first()
if not user: # create if not created already
expiration_date_token = datetime.date.today() + \
datetime.timedelta(
days=flask.current_app.config["API_TOKEN_EXPIRATION"])
copr64 = base64.b64encode("copr") + "##"
- user = models.User(openid_name=resp.identity_url, mail=resp.email,
+ user = models.User(username=username, mail=resp.email,
timezone=resp.timezone,
api_login=copr64 + helpers.generate_api_token(
app.config["API_TOKEN_LENGTH"] -
len(copr64)),
diff --git a/frontend/coprs_frontend/tests/coprs_test_case.py
b/frontend/coprs_frontend/tests/coprs_test_case.py
index 179555c..54fe7fe 100644
--- a/frontend/coprs_frontend/tests/coprs_test_case.py
+++ b/frontend/coprs_frontend/tests/coprs_test_case.py
@@ -48,18 +48,18 @@ class CoprsTestCase(object):
@pytest.fixture
def f_users(self):
self.u1 = models.User(
-
openid_name=u"http://user1.id.fedoraproject.org/",
+ username=u"user1",
proven=False,
admin=True,
mail="user1(a)foo.bar")
self.u2 = models.User(
-
openid_name=u"http://user2.id.fedoraproject.org/",
+ username=u"user2",
proven=False,
mail="user2(a)spam.foo")
self.u3 = models.User(
-
openid_name=u"http://user3.id.fedoraproject.org/",
+ username=u"user3",
proven=False,
mail="baz(a)bar.bar")
@@ -198,7 +198,7 @@ class TransactionDecorator(object):
with self.tc as test_client:
with c.session_transaction() as session:
- session['openid'] = self.u.openid_name
+ session['openid'] = self.u.username
where 'u' stands for any user from 'f_users' fixture, use this to
decorate
your test function:
@@ -209,7 +209,7 @@ class TransactionDecorator(object):
# you can also access object 'test_client' through
'self.test_client'
where decorator parameter ''u'' stands for string representation of
any
- user from 'f_users' fixture from which you wish to store
'openid_name'.
+ user from 'f_users' fixture from which you wish to store 'username'.
Please note that you **must** include 'f_users' fixture in decorated
function parameters.
@@ -223,6 +223,6 @@ class TransactionDecorator(object):
def wrapper(fn, fn_self, *args):
with fn_self.tc as fn_self.test_client:
with fn_self.test_client.session_transaction() as session:
- session["openid"] = getattr(fn_self,
self.user).openid_name
+ session["openid"] = getattr(fn_self, self.user).username
return fn(fn_self, *args)
return decorator.decorator(wrapper, fn)
diff --git a/frontend/coprs_frontend/tests/test_views/test_admin/test_admin_general.py
b/frontend/coprs_frontend/tests/test_views/test_admin/test_admin_general.py
index 60eba07..7489374 100644
--- a/frontend/coprs_frontend/tests/test_views/test_admin/test_admin_general.py
+++ b/frontend/coprs_frontend/tests/test_views/test_admin/test_admin_general.py
@@ -9,7 +9,7 @@ class TestAdminLogin(CoprsTestCase):
def test_nonadmin_cant_login(self, f_users, f_db):
with self.tc as c:
with c.session_transaction() as s:
- s["openid"] = self.u2.openid_name
+ s["openid"] = self.u2.username
r = c.get("/admin/", follow_redirects=True)
assert self.text_to_check not in r.data
@@ -17,7 +17,7 @@ class TestAdminLogin(CoprsTestCase):
def test_admin_can_login(self, f_users, f_db):
with self.tc as c:
with c.session_transaction() as s:
- s["openid"] = self.u1.openid_name
+ s["openid"] = self.u1.username
r = c.get("/admin/", follow_redirects=True)
assert self.text_to_check in r.data