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.

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

Implement Browser Analytics #1329

Merged
merged 3 commits into from
Jul 1, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"babel-preset-es2015-native-modules": "6.6.0",
"babel-register": "6.7.2",
"clipboard": "1.5.10",
"cookie": "0.3.1",
"del": "2.2.0",
"exports-loader": "0.6.3",
"font-awesome": "4.5.0",
Expand Down
4 changes: 3 additions & 1 deletion tests/unit/accounts/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import uuid

import pretend

from zope.interface.verify import verifyClass
Expand Down Expand Up @@ -60,7 +62,7 @@ def test_find_userid_existing_user(self, db_session):

def test_check_password_nonexistant_user(self, db_session):
service = services.DatabaseUserService(db_session)
assert not service.check_password(1, None)
assert not service.check_password(uuid.uuid4(), None)

def test_check_password_invalid(self, db_session):
user = UserFactory.create()
Expand Down
10 changes: 6 additions & 4 deletions tests/unit/accounts/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
# limitations under the License.

import datetime
import uuid

import freezegun
import pretend
Expand Down Expand Up @@ -118,8 +119,9 @@ def test_post_validate_redirects(self, monkeypatch, pyramid_request,

new_session = {}

user_id = uuid.uuid4()
user_service = pretend.stub(
find_userid=pretend.call_recorder(lambda username: 1),
find_userid=pretend.call_recorder(lambda username: user_id),
update_user=pretend.call_recorder(lambda *a, **kw: None),
)
pyramid_request.find_service = pretend.call_recorder(
Expand All @@ -134,7 +136,7 @@ def test_post_validate_redirects(self, monkeypatch, pyramid_request,
)

pyramid_request.set_property(
lambda r: 1234 if with_user else None,
lambda r: str(uuid.uuid4()) if with_user else None,
name="unauthenticated_userid",
)

Expand All @@ -161,15 +163,15 @@ def test_post_validate_redirects(self, monkeypatch, pyramid_request,

assert user_service.find_userid.calls == [pretend.call("theuser")]
assert user_service.update_user.calls == [
pretend.call(1, last_login=now),
pretend.call(user_id, last_login=now),
]

if with_user:
assert new_session == {}
else:
assert new_session == {"a": "b", "foo": "bar"}

assert remember.calls == [pretend.call(pyramid_request, 1)]
assert remember.calls == [pretend.call(pyramid_request, str(user_id))]
assert pyramid_request.session.invalidate.calls == [pretend.call()]
assert pyramid_request.find_service.calls == [
pretend.call(IUserService, context=None),
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_csp.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def test_includeme():
],
"referrer": ["origin-when-cross-origin"],
"reflected-xss": ["block"],
"script-src": ["'self'"],
"script-src": ["'self'", "www.google-analytics.com"],
"style-src": ["'self'", "fonts.googleapis.com"],
},
})
Expand Down
12 changes: 4 additions & 8 deletions warehouse/accounts/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
Boolean, DateTime, Integer, String,
)
from sqlalchemy import orm, select, sql
from sqlalchemy.dialects.postgresql import UUID
from sqlalchemy.orm.exc import NoResultFound
from sqlalchemy.ext.hybrid import hybrid_property

Expand All @@ -38,7 +39,7 @@ def __getitem__(self, username):
raise KeyError from None


class User(SitemapMixin, db.ModelBase):
class User(SitemapMixin, db.Model):

__tablename__ = "accounts_user"
__table_args__ = (
Expand All @@ -51,7 +52,6 @@ class User(SitemapMixin, db.ModelBase):

__repr__ = make_repr("username")

id = Column(Integer, primary_key=True, nullable=False)
username = Column(CIText, nullable=False, unique=True)
name = Column(String(length=100), nullable=False)
password = Column(String(length=128), nullable=False)
Expand Down Expand Up @@ -104,12 +104,8 @@ class Email(db.ModelBase):

id = Column(Integer, primary_key=True, nullable=False)
user_id = Column(
Integer,
ForeignKey(
"accounts_user.id",
deferrable=True,
initially="DEFERRED",
),
UUID(as_uuid=True),
ForeignKey("accounts_user.id", deferrable=True, initially="DEFERRED"),
nullable=False,
)
email = Column(String(length=254), nullable=False)
Expand Down
32 changes: 29 additions & 3 deletions warehouse/accounts/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import datetime

from pyblake2 import blake2b
from pyramid.httpexceptions import HTTPMovedPermanently, HTTPSeeOther
from pyramid.security import remember, forget
from pyramid.view import view_config
Expand All @@ -25,6 +26,9 @@
from warehouse.utils.http import is_safe_url


USER_ID_INSECURE_COOKIE = "user_id__insecure"


@view_config(
route_name="accounts.profile",
renderer="accounts/profile.html",
Expand Down Expand Up @@ -91,7 +95,23 @@ def login(request, redirect_field_name=REDIRECT_FIELD_NAME,

# Now that we're logged in we'll want to redirect the user to either
# where they were trying to go originally, or to the default view.
return HTTPSeeOther(redirect_to, headers=dict(headers))
resp = HTTPSeeOther(redirect_to, headers=dict(headers))

# We'll use this cookie so that client side javascript can Determine
# the actual user ID (not username, user ID). This is *not* a security
# sensitive context and it *MUST* not be used where security matters.
#
# We'll also hash this value just to avoid leaking the actual User IDs
# here, even though it really shouldn't matter.
resp.set_cookie(
USER_ID_INSECURE_COOKIE,
blake2b(
str(userid).encode("ascii"),
person=b"warehouse.userid",
).hexdigest().lower(),
)

return resp

return {
"form": form,
Expand Down Expand Up @@ -141,7 +161,13 @@ def logout(request, redirect_field_name=REDIRECT_FIELD_NAME):

# Now that we're logged out we'll want to redirect the user to either
# where they were originally, or to the default view.
return HTTPSeeOther(redirect_to, headers=dict(headers))
resp = HTTPSeeOther(redirect_to, headers=dict(headers))

# Ensure that we delete our user_id__insecure cookie, since the user is
# no longer logged in.
resp.delete_cookie(USER_ID_INSECURE_COOKIE)

return resp

return {"redirect": {"field": REDIRECT_FIELD_NAME, "data": redirect_to}}

Expand Down Expand Up @@ -213,7 +239,7 @@ def _login_user(request, userid):
request.session.update(data)

# Remember the userid using the authentication policy.
headers = remember(request, userid)
headers = remember(request, str(userid))

# Cycle the CSRF token since we've crossed an authentication boundary
# and we don't want to continue using the old one.
Expand Down
1 change: 1 addition & 0 deletions warehouse/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ def configure(settings=None):
maybe_set(settings, "camo.url", "CAMO_URL")
maybe_set(settings, "camo.key", "CAMO_KEY")
maybe_set(settings, "docs.url", "DOCS_URL")
maybe_set(settings, "ga.tracking_id", "GA_TRACKING_ID")
maybe_set_compound(settings, "files", "backend", "FILES_BACKEND")
maybe_set_compound(settings, "origin_cache", "backend", "ORIGIN_CACHE")

Expand Down
2 changes: 1 addition & 1 deletion warehouse/csp.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def includeme(config):
],
"referrer": ["origin-when-cross-origin"],
"reflected-xss": ["block"],
"script-src": [SELF],
"script-src": [SELF, "www.google-analytics.com"],
"style-src": [SELF, "fonts.googleapis.com"],
},
})
Expand Down
9 changes: 3 additions & 6 deletions warehouse/legacy/tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
UniqueConstraint,
Boolean, Date, DateTime, Integer, LargeBinary, String, Text,
)
from sqlalchemy.dialects.postgresql import UUID

from warehouse import db

Expand All @@ -34,12 +35,8 @@
Column("id", Integer(), primary_key=True, nullable=False),
Column(
"user_id",
Integer(),
ForeignKey(
"accounts_user.id",
deferrable=True,
initially="DEFERRED",
),
UUID(as_uuid=True),
ForeignKey("accounts_user.id", deferrable=True, initially="DEFERRED"),
nullable=False,
),
Column("key_id", CIText(), nullable=False),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""
Switch to a UUID based primary key for User

Revision ID: 8c8be2c0e69e
Revises: 039f45e2dbf9
Create Date: 2016-07-01 18:20:42.072664
"""


import sqlalchemy as sa

from alembic import op
from sqlalchemy.dialects import postgresql


revision = "8c8be2c0e69e"
down_revision = "039f45e2dbf9"


def upgrade():
# Add a new column which is going to hold all of our new IDs for this table
# with a temporary name until we can rename it.
op.add_column(
"accounts_user",
sa.Column(
"new_id",
postgresql.UUID(as_uuid=True),
server_default=sa.text("gen_random_uuid()"),
nullable=False,
),
)

# Add a column to tables that refer to accounts_user so they can be updated
# to refer to it.
op.add_column(
"accounts_email",
sa.Column("new_user_id", postgresql.UUID(as_uuid=True), nullable=True),
)
op.add_column(
"accounts_gpgkey",
sa.Column("new_user_id", postgresql.UUID(as_uuid=True), nullable=True),
)

# Update our referring tables so that their new column points to the
# correct user account.
op.execute(
""" UPDATE accounts_email
SET new_user_id = accounts_user.new_id
FROM accounts_user
WHERE accounts_email.user_id = accounts_user.id
"""
)
op.execute(
""" UPDATE accounts_gpgkey
SET new_user_id = accounts_user.new_id
FROM accounts_user
WHERE accounts_gpgkey.user_id = accounts_user.id
"""
)

# Disallow any NULL values in our referring tables
op.alter_column("accounts_email", "new_user_id", nullable=False)
op.alter_column("accounts_gpgkey", "new_user_id", nullable=False)

# Delete our existing fields and move our new fields into their old places.
op.drop_constraint("accounts_email_user_id_fkey", "accounts_email")
op.drop_column("accounts_email", "user_id")
op.alter_column("accounts_email", "new_user_id", new_column_name="user_id")

op.drop_constraint("accounts_gpgkey_user_id_fkey", "accounts_gpgkey")
op.drop_column("accounts_gpgkey", "user_id")
op.alter_column(
"accounts_gpgkey", "new_user_id", new_column_name="user_id")

# Switch the primary key from the old to the new field, drop the old name,
# and rename the new field into it's place.
op.drop_constraint("accounts_user_pkey", "accounts_user")
op.create_primary_key(None, "accounts_user", ["new_id"])
op.drop_column("accounts_user", "id")
op.alter_column("accounts_user", "new_id", new_column_name="id")

# Finally, Setup our foreign key constraints for our referring tables.
op.create_foreign_key(
None,
"accounts_email",
"accounts_user",
["user_id"],
["id"],
deferrable=True,
)
op.create_foreign_key(
None,
"accounts_gpgkey",
"accounts_user",
["user_id"],
["id"],
deferrable=True,
)


def downgrade():
raise RuntimeError("Order No. 227 - Ни шагу назад!")
4 changes: 4 additions & 0 deletions warehouse/static/js/warehouse/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,17 @@ import "babel-polyfill";
import docReady from "warehouse/utils/doc-ready";

// Import our utility functions
import Analytics from "warehouse/utils/analytics";
import HTMLInclude from "warehouse/utils/html-include";
import * as formUtils from "warehouse/utils/forms";
import Clipboard from "clipboard";

// Kick off the client side HTML includes.
docReady(HTMLInclude);

// Trigger our analytics code.
docReady(Analytics);

// Handle the JS based automatic form submission.
docReady(formUtils.submitTriggers);

Expand Down
Loading