Add display_name field to users to specify a new name within the application, without changing the login name

api/migrations/versions/99a3d2eb08f9_add_user_display_name.py

@@ -0,0 +1,26 @@
+"""add user display_name
+
+Revision ID: 99a3d2eb08f9
+Revises: a9627f63fbed
+Create Date: 2022-09-13 07:30:18.747880
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = "99a3d2eb08f9"
+down_revision = "a9627f63fbed"
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    op.add_column(
+        "user", sa.Column("display_name", sa.String, nullable=True), schema="public"
+    )
+
+
+def downgrade():
+    op.drop_column("user", "display_name", schema="public")

api/obs/api/db.py

@@ -3,7 +3,7 @@ from contextvars import ContextVar
 from contextlib import asynccontextmanager
 from datetime import datetime
 import os
-from os.path import join, dirname
+from os.path import exists, join, dirname
 from json import loads
 import re
 import math
@@ -12,6 +12,7 @@ import random
 import string
 import secrets
 from slugify import slugify
+import logging
 
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.ext.asyncio import AsyncSession
@@ -37,6 +38,8 @@ from sqlalchemy import (
 from sqlalchemy.dialects.postgresql import HSTORE, UUID
 
 
+log = logging.getLogger(__name__)
+
 Base = declarative_base()
 
 
@@ -351,6 +354,7 @@ class User(Base):
     updated_at = Column(DateTime, nullable=False, server_default=NOW, onupdate=NOW)
     sub = Column(String, unique=True, nullable=False)
     username = Column(String, unique=True, nullable=False)
+    display_name = Column(String, nullable=True)
     email = Column(String, nullable=False)
     bio = Column(TEXT)
     image = Column(String)
@@ -371,11 +375,38 @@ class User(Base):
         self.api_key = secrets.token_urlsafe(24)
 
     def to_dict(self, for_user_id=None):
-        return {
-            "username": self.username,
+        result = {
+            "id": self.id,
+            "displayName": self.display_name or self.username,
             "bio": self.bio,
             "image": self.image,
         }
+        if for_user_id == self.id:
+            result["username"] = self.username
+        return result
+
+    async def rename(self, config, new_name):
+        old_name = self.username
+
+        renames = [
+            (join(basedir, old_name), join(basedir, new_name))
+            for basedir in [config.PROCESSING_OUTPUT_DIR, config.TRACKS_DIR]
+        ]
+
+        for src, dst in renames:
+            if exists(dst):
+                raise FileExistsError(
+                    f"cannot move {src!r} to {dst!r}, destination exists"
+                )
+
+        for src, dst in renames:
+            if not exists(src):
+                log.debug("Rename user %s: Not moving %s, not found", self.id, src)
+            else:
+                log.info("Rename user %s: Moving %s to %s", self.id, src, dst)
+                os.rename(src, dst)
+
+        self.username = new_name
 
 
 class Comment(Base):
@@ -403,7 +434,10 @@ class Comment(Base):
 
 Comment.author = relationship("User", back_populates="authored_comments")
 User.authored_comments = relationship(
-    "Comment", order_by=Comment.created_at, back_populates="author", passive_deletes=True
+    "Comment",
+    order_by=Comment.created_at,
+    back_populates="author",
+    passive_deletes=True,
 )
 
 Track.author = relationship("User", back_populates="authored_tracks")
@@ -418,7 +452,10 @@ Track.comments = relationship(
 
 OvertakingEvent.track = relationship("Track", back_populates="overtaking_events")
 Track.overtaking_events = relationship(
-    "OvertakingEvent", order_by=OvertakingEvent.time, back_populates="track", passive_deletes=True
+    "OvertakingEvent",
+    order_by=OvertakingEvent.time,
+    back_populates="track",
+    passive_deletes=True,
 )
 
 

api/obs/api/routes/login.py

@@ -1,5 +1,6 @@
 import asyncio
 import logging
+import re
 
 from requests.exceptions import RequestException
 
@@ -91,6 +92,15 @@ async def login_redirect(req):
     preferred_username = userinfo["preferred_username"]
     email = userinfo.get("email")
 
+    clean_username = re.sub(r"[^a-zA-Z0-9_.-]", "", preferred_username)
+    if clean_username != preferred_username:
+        log.warning(
+            "Username %r contained invalid characters and was changed to %r",
+            preferred_username,
+            clean_username,
+        )
+        preferred_username = clean_username
+
     if email is None:
         raise ValueError(
             "user has no email set, please configure keycloak to require emails"
@@ -128,16 +138,20 @@ async def login_redirect(req):
         user = User(sub=sub, username=preferred_username, email=email)
         req.ctx.db.add(user)
     else:
-        log.info("Logged in known user (id: %s, sub: %s).", user.id, user.sub)
+        log.info(
+            "Logged in known user (id: %s, sub: %s, %s).",
+            user.id,
+            user.sub,
+            preferred_username,
+        )
 
         if email != user.email:
             log.debug("Updating user (id: %s) email from auth system.", user.id)
             user.email = email
 
-        # TODO: re-add username change when we can safely rename users
-        # if preferred_username != user.username:
-        #     log.debug("Updating user (id: %s) username from auth system.", user.id)
-        #     user.username = preferred_username
+        if preferred_username != user.username:
+            log.debug("Updating user (id: %s) username from auth system.", user.id)
+            await user.rename(req.app.config, preferred_username)
 
     await req.ctx.db.commit()
 
@@ -156,4 +170,4 @@ async def logout(req):
     auth_req = client.construct_EndSessionRequest(state=session["state"])
     logout_url = auth_req.request(client.end_session_endpoint)
 
-    return redirect(logout_url+f"&redirect_uri={req.ctx.api_url}/logout")
+    return redirect(logout_url + f"&redirect_uri={req.ctx.api_url}/logout")

api/obs/api/routes/tiles.py

@@ -66,12 +66,9 @@ def get_filter_options(
         * start (datetime|None)
         * end (datetime|None)
     """
-    user_id = None
-    username = req.ctx.get_single_arg("user", default=None)
-    if username is not None:
-        if req.ctx.user is None or req.ctx.user.username != username:
-            raise Forbidden()
-        user_id = req.ctx.user.id
+    user_id = req.ctx.get_single_arg("user", default=None, convert=int)
+    if user_id is not None and (req.ctx.user is None or req.ctx.user.id != user_id):
+        raise Forbidden()
 
     parse_date = lambda s: dateutil.parser.parse(s)
     start = req.ctx.get_single_arg("start", default=None, convert=parse_date)

api/obs/api/routes/tracks.py

@@ -63,13 +63,13 @@ async def _return_tracks(req, extend_query, limit, offset):
 async def get_tracks(req):
     limit = req.ctx.get_single_arg("limit", default=20, convert=int)
     offset = req.ctx.get_single_arg("offset", default=0, convert=int)
-    author = req.ctx.get_single_arg("author", default=None, convert=str)
+    # author = req.ctx.get_single_arg("author", default=None, convert=int)
 
     def extend_query(q):
         q = q.where(Track.public)
 
-        if author is not None:
-            q = q.where(User.username == author)
+        # if author is not None:
+        #     q = q.where(Track.author_id == author)
 
         return q
 

api/obs/api/routes/users.py

@@ -12,7 +12,9 @@ from obs.api import __version__ as version
 
 def user_to_json(user):
     return {
+        "id": user.id,
         "username": user.username,
+        "displayName": user.display_name,
         "email": user.email,
         "bio": user.bio,
         "image": user.image,
@@ -36,6 +38,9 @@ async def put_user(req):
         if key in data and isinstance(data[key], (str, type(None))):
             setattr(user, key, data[key])
 
+    if "displayName" in data:
+        user.display_name = data["displayName"] or None
+
     if "areTracksVisibleForAll" in data:
         user.are_tracks_visible_for_all = bool(data["areTracksVisibleForAll"])
 

frontend/src/components/Avatar/index.tsx

@@ -1,39 +1,42 @@
-import React from 'react'
-import {Comment} from 'semantic-ui-react'
-import classnames from 'classnames'
+import React from "react";
+import { Comment } from "semantic-ui-react";
+import classnames from "classnames";
 
-import './styles.less'
+import "./styles.less";
 
 function hashCode(s) {
-  let hash = 0
+  let hash = 0;
   for (let i = 0; i < s.length; i++) {
-    hash = (hash << 5) - hash + s.charCodeAt(i)
-    hash |= 0
+    hash = (hash << 5) - hash + s.charCodeAt(i);
+    hash |= 0;
   }
-  return hash
+  return hash;
 }
 
 function getColor(s) {
-  const h = Math.floor(hashCode(s)) % 360
-  return `hsl(${h}, 50%, 50%)`
+  const h = Math.floor(hashCode(s)) % 360;
+  return `hsl(${h}, 50%, 50%)`;
 }
 
-export default function Avatar({user, className}) {
-  const {image, username} = user || {}
+export default function Avatar({ user, className }) {
+  const { image, displayName } = user || {};
 
   if (image) {
-    return <Comment.Avatar src={image} className={className} />
+    return <Comment.Avatar src={image} className={className} />;
   }
 
-  if (!username) {
-    return <div className={classnames(className, 'avatar', 'empty-avatar')} />
+  if (!displayName) {
+    return <div className={classnames(className, "avatar", "empty-avatar")} />;
   }
 
-  const color = getColor(username)
+  const color = getColor(displayName);
 
   return (
-    <div className={classnames(className, 'avatar', 'text-avatar')} style={{background: color}}>
-      {username && <span>{username[0]}</span>}
+    <div
+      className={classnames(className, "avatar", "text-avatar")}
+      style={{ background: color }}
+    >
+      {displayName && <span>{displayName[0]}</span>}
     </div>
-  )
+  );
 }

frontend/src/pages/MapPage/index.tsx

@@ -169,7 +169,7 @@ function MapPage({ login }) {
     const query = new URLSearchParams();
     if (login) {
       if (mapConfig.filters.currentUser) {
-        query.append("user", login.username);
+        query.append("user", login.id);
       }
 
       if (mapConfig.filters.dateMode === "range") {

frontend/src/pages/SettingsPage.tsx

@@ -68,18 +68,37 @@ const SettingsPage = connect((state) => ({ login: state.login }), { setLogin })(
             <Grid.Column width={8}>
               <Header as="h2">{t("SettingsPage.profile.title")}</Header>
 
-              <Message info>{t("SettingsPage.profile.publicNotice")}</Message>
-
               <Form onSubmit={handleSubmit(onSave)} loading={loading}>
-                <Ref innerRef={findInput(register)}>
-                  <Form.Input
-                    error={errors?.username}
-                    label={t("SettingsPage.profile.username.label")}
-                    name="username"
-                    defaultValue={login.username}
-                    disabled
-                  />
-                </Ref>
+                <Form.Field error={errors?.username}>
+                  <label>{t("SettingsPage.profile.username.label")}</label>
+                  <Ref innerRef={findInput(register)}>
+                    <Input
+                      name="username"
+                      defaultValue={login.username}
+                      disabled
+                    />
+                  </Ref>
+                  <small>{t("SettingsPage.profile.username.hint")}</small>
+                </Form.Field>
+
+                <Message info visible>
+                  {t("SettingsPage.profile.publicNotice")}
+                </Message>
+
+                <Form.Field error={errors?.displayName}>
+                  <label>{t("SettingsPage.profile.displayName.label")}</label>
+                  <Ref innerRef={findInput(register)}>
+                    <Input
+                      name="displayName"
+                      defaultValue={login.displayName}
+                      placeholder={login.username}
+                    />
+                  </Ref>
+                  <small>
+                    {t("SettingsPage.profile.displayName.fallbackNotice")}
+                  </small>
+                </Form.Field>
+
                 <Form.Field error={errors?.bio}>
                   <label>{t("SettingsPage.profile.bio.label")}</label>
                   <Ref innerRef={register}>
@@ -103,7 +122,7 @@ const SettingsPage = connect((state) => ({ login: state.login }), { setLogin })(
 
               <Divider />
 
-              <Stats user={login.username} />
+              <Stats user={login.id} />
             </Grid.Column>
           </Grid.Row>
         </Grid>
@@ -140,7 +159,7 @@ function CopyInput({ value, ...props }) {
       }
       position="top right"
       open={success != null}
-      content={success ? t('general.copied') : t('general.copyError')}
+      content={success ? t("general.copied") : t("general.copyError")}
     />
   );
 }

frontend/src/pages/TrackEditor.tsx

@@ -76,7 +76,7 @@ const TrackEditor = connect((state) => ({ login: state.login }))(
     );
 
     const loading = busy || track == null;
-    const isAuthor = login?.username === track?.author?.username;
+    const isAuthor = login?.id === track?.author?.id;
 
     // Navigate to track detials if we are not the author
     React.useEffect(() => {

frontend/src/pages/TrackPage/TrackComments.tsx

@@ -60,7 +60,9 @@ export default function TrackComments({
           <Comment key={comment.id}>
             <Avatar user={comment.author} />
             <Comment.Content>
-              <Comment.Author as="a">{comment.author.username}</Comment.Author>
+              <Comment.Author as="a">
+                {comment.author.displayName}
+              </Comment.Author>
               <Comment.Metadata>
                 <div>
                   <FormattedDate date={comment.createdAt} relative />
@@ -69,7 +71,7 @@ export default function TrackComments({
               <Comment.Text>
                 <Markdown>{comment.body}</Markdown>
               </Comment.Text>
-              {login?.username === comment.author.username && (
+              {login?.id === comment.author.id && (
                 <Comment.Actions>
                   <Comment.Action
                     onClick={(e) => {
@@ -77,7 +79,7 @@ export default function TrackComments({
                       e.preventDefault();
                     }}
                   >
-                  {t('general.delete')}
+                    {t("general.delete")}
                   </Comment.Action>
                 </Comment.Actions>
               )}

frontend/src/pages/TrackPage/index.tsx

@@ -246,7 +246,7 @@ const TrackPage = connect((state) => ({ login: state.login }))(
       [slug]
     );
 
-    const isAuthor = login?.username === data?.track?.author?.username;
+    const isAuthor = login?.id === data?.track?.author?.id;
 
     const { track, trackData, comments } = data || {};
 
@@ -355,7 +355,7 @@ const TrackPage = connect((state) => ({ login: state.login }))(
           onConfirm={hideDownloadError}
           header={t("TrackPage.downloadFailed")}
           content={String(downloadError)}
-          confirmButton={t('general.ok')}
+          confirmButton={t("general.ok")}
         />
       </Page>
     );

frontend/src/pages/TracksPage.tsx

@@ -1,49 +1,65 @@
-import React, {useCallback} from 'react'
-import {connect} from 'react-redux'
-import {Button, Message, Item, Header, Loader, Pagination, Icon} from 'semantic-ui-react'
-import {useObservable} from 'rxjs-hooks'
-import {Link} from 'react-router-dom'
-import {of, from, concat} from 'rxjs'
-import {map, switchMap, distinctUntilChanged} from 'rxjs/operators'
-import _ from 'lodash'
-import {useTranslation, Trans as Translate} from 'react-i18next'
+import React, { useCallback } from "react";
+import { connect } from "react-redux";
+import {
+  Button,
+  Message,
+  Item,
+  Header,
+  Loader,
+  Pagination,
+  Icon,
+} from "semantic-ui-react";
+import { useObservable } from "rxjs-hooks";
+import { Link } from "react-router-dom";
+import { of, from, concat } from "rxjs";
+import { map, switchMap, distinctUntilChanged } from "rxjs/operators";
+import _ from "lodash";
+import { useTranslation, Trans as Translate } from "react-i18next";
 
-import type {Track} from 'types'
-import {Avatar, Page, StripMarkdown, FormattedDate, Visibility} from 'components'
-import api from 'api'
-import {useQueryParam} from 'query'
+import type { Track } from "types";
+import {
+  Avatar,
+  Page,
+  StripMarkdown,
+  FormattedDate,
+  Visibility,
+} from "components";
+import api from "api";
+import { useQueryParam } from "query";
 
-function TrackList({privateTracks}: {privateTracks: boolean}) {
-  const [page, setPage] = useQueryParam<number>('page', 1, Number)
+function TrackList({ privateTracks }: { privateTracks: boolean }) {
+  const [page, setPage] = useQueryParam<number>("page", 1, Number);
 
-  const pageSize = 10
+  const pageSize = 10;
 
   const data: {
-    tracks: Track[]
-    trackCount: number
+    tracks: Track[];
+    trackCount: number;
   } | null = useObservable(
     (_$, inputs$) =>
       inputs$.pipe(
         map(([page, privateTracks]) => {
-          const url = '/tracks' + (privateTracks ? '/feed' : '')
-          const query = {limit: pageSize, offset: pageSize * (page - 1)}
-          return {url, query}
+          const url = "/tracks" + (privateTracks ? "/feed" : "");
+          const query = { limit: pageSize, offset: pageSize * (page - 1) };
+          return { url, query };
         }),
         distinctUntilChanged(_.isEqual),
-        switchMap((request) => concat(of(null), from(api.get(request.url, {query: request.query}))))
+        switchMap((request) =>
+          concat(of(null), from(api.get(request.url, { query: request.query })))
+        )
       ),
     null,
     [page, privateTracks]
-  )
+  );
 
-  const {tracks, trackCount} = data || {tracks: [], trackCount: 0}
-  const loading = !data
-  const totalPages = Math.ceil(trackCount / pageSize)
-  const {t} = useTranslation()
+  const { tracks, trackCount } = data || { tracks: [], trackCount: 0 };
+  const loading = !data;
+  const totalPages = Math.ceil(trackCount / pageSize);
+  const { t } = useTranslation();
 
   return (
     <div>
-      <Loader content={t('general.loading')} active={loading} />
+      <Loader content={t("general.loading")} active={loading} />
       {!loading && totalPages > 1 && (
         <Pagination
           activePage={page}
@@ -55,14 +71,14 @@ function TrackList({privateTracks}: {privateTracks: boolean}) {
       {tracks && tracks.length ? (
         <Item.Group divided>
           {tracks.map((track: Track) => (
-            <TrackListItem key={track.slug} {...{track, privateTracks}} />
+            <TrackListItem key={track.slug} {...{ track, privateTracks }} />
           ))}
         </Item.Group>
       ) : (
         <NoPublicTracksMessage />
       )}
     </div>
-  )
+  );
 }
 
 export function NoPublicTracksMessage() {
@@ -72,27 +88,27 @@ export function NoPublicTracksMessage() {
         No public tracks yet. <Link to="/upload">Upload the first!</Link>
       </Translate>
     </Message>
-  )
+  );
 }
 
-function maxLength(t: string|null, max: number): string|null {
+function maxLength(t: string | null, max: number): string | null {
   if (t && t.length > max) {
-    return t.substring(0, max) + ' ...'
+    return t.substring(0, max) + " ...";
   } else {
-    return t
+    return t;
   }
 }
 
 const COLOR_BY_STATUS = {
-  error: 'red',
-  complete: 'green',
-  created: 'gray',
-  queued: 'orange',
-  processing: 'orange',
-}
+  error: "red",
+  complete: "green",
+  created: "gray",
+  queued: "orange",
+  processing: "orange",
+};
 
-export function TrackListItem({track, privateTracks = false}) {
-  const {t} = useTranslation()
+export function TrackListItem({ track, privateTracks = false }) {
+  const { t } = useTranslation();
 
   return (
     <Item key={track.slug}>
@@ -101,10 +117,14 @@ export function TrackListItem({track, privateTracks = false}) {
       </Item.Image>
       <Item.Content>
         <Item.Header as={Link} to={`/tracks/${track.slug}`}>
-          {track.title || t('general.unnamedTrack')}
+          {track.title || t("general.unnamedTrack")}
         </Item.Header>
         <Item.Meta>
-          {privateTracks ? null : <span>{t('TracksPage.createdBy', {author: track.author.username})}</span>}
+          {privateTracks ? null : (
+            <span>
+              {t("TracksPage.createdBy", { author: track.author.displayName })}
+            </span>
+          )}
           <span>
             <FormattedDate date={track.createdAt} />
           </span>
@@ -116,45 +136,57 @@ export function TrackListItem({track, privateTracks = false}) {
           <Item.Extra>
             <Visibility public={track.public} />
 
-            <span style={{marginLeft: '1em'}}>
-              <Icon color={COLOR_BY_STATUS[track.processingStatus]} name="bolt" fitted />
-              {' '}
+            <span style={{ marginLeft: "1em" }}>
+              <Icon
+                color={COLOR_BY_STATUS[track.processingStatus]}
+                name="bolt"
+                fitted
+              />{" "}
               {t(`TracksPage.processing.${track.processingStatus}`)}
             </span>
           </Item.Extra>
         )}
       </Item.Content>
     </Item>
-  )
+  );
 }
 
-function UploadButton({navigate, ...props}) {
-  const {t} = useTranslation()
+function UploadButton({ navigate, ...props }) {
+  const { t } = useTranslation();
   const onClick = useCallback(
     (e) => {
-      e.preventDefault()
-      navigate()
+      e.preventDefault();
+      navigate();
     },
     [navigate]
-  )
+  );
   return (
-    <Button onClick={onClick} {...props} color="green" style={{float: 'right'}}>
-      {t('TracksPage.upload')}
+    <Button
+      onClick={onClick}
+      {...props}
+      color="green"
+      style={{ float: "right" }}
+    >
+      {t("TracksPage.upload")}
     </Button>
-  )
+  );
 }
 
-const TracksPage = connect((state) => ({login: (state as any).login}))(function TracksPage({login, privateTracks}) {
-  const {t} = useTranslation()
-  const title = privateTracks ? t('TracksPage.titleUser') : t('TracksPage.titlePublic')
+const TracksPage = connect((state) => ({ login: (state as any).login }))(
+  function TracksPage({ login, privateTracks }) {
+    const { t } = useTranslation();
+    const title = privateTracks
+      ? t("TracksPage.titleUser")
+      : t("TracksPage.titlePublic");
 
-  return (
-    <Page title={title}>
-      <Header as='h2'>{title}</Header>
-      {privateTracks && <Link component={UploadButton} to="/upload" />}
-      <TrackList {...{privateTracks}} />
-    </Page>
-  )
-})
+    return (
+      <Page title={title}>
+        <Header as="h2">{title}</Header>
+        {privateTracks && <Link component={UploadButton} to="/upload" />}
+        <TrackList {...{ privateTracks }} />
+      </Page>
+    );
+  }
+);
 
-export default TracksPage
+export default TracksPage;

frontend/src/translations/de.yaml

@@ -206,9 +206,15 @@ SettingsPage:
 
   profile:
     title: Mein Profil
-    publicNotice: All diese Informationen sind öffentlich.
+    publicNotice: Alle Informationen ab hier sind öffentlich.
     username:
       label: Kontoname
+      hint: >
+        Der Kontoname kann beim Login-Dienst geändert werden, und wird beim
+        nächsten Login übernommen.
+    displayName:
+      label: Anzeigename
+      fallbackNotice: Sofern kein Anzeigename gewählt ist wird der Kontoname öffentlich angezeigt.
     bio:
       label: Bio
     avatarUrl:

frontend/src/translations/en.yaml

@@ -210,9 +210,15 @@ SettingsPage:
 
   profile:
     title: My profile
-    publicNotice: All of this information is public.
+    publicNotice: All of the information below is public.
     username:
       label: Username
+      hint: >
+        The username can be changed at the login provider and will be applied
+        when logging in the next time.
+    displayName:
+      label: Display name
+      fallbackNotice: As long as no display name is chosen, the username is shown publicly.
     bio:
       label: Bio
     avatarUrl:

frontend/src/types.ts

@@ -1,7 +1,8 @@
 import type {FeatureCollection, Feature, LineString, Point} from 'geojson'
 
 export type UserProfile = {
-  username: string
+  id: number | string
+  displayName: string
   image?: string | null
   bio?: string | null
 }