[ccoenen]

I am currently implementing a profile page that would let you edit your display name (henceforth: name) and possibly edit your password. It is far from finished, and I did a little bit of groundwork for future pages along the way. Those should get their own pull requests, probably. Also this will contain commits not directly related to either one, more specifically those from PR #947 )

Questions:

• with more and more pages, the "old" way of putting all the variables in all the render calls does not work any more. Express has app.locals which are available in the template files. Does this work for you? I find this much cleaner. All those changes are now proposed as PR in removing global site layout vars from individual routers #1035
• Did I get all the places, where those variables are used? Does EJS have a way to detect used but unset variables (like a strict mode for EJS, perhaps)?
• We could put "delete user" on that page instead of the index-page-menu. Opinions?

TODO:

• The header contains everything and the kitchen sink from the note editor. This needs to be fixed
• user name will have to be sanitized. I would hate to restrict it to [a-z] (even though this would strictly not be a regression, it does feel like one). I have no experience with express/sequelize/ejs, so I would value your input - even just in form of documentation or a how-to, so I don't introduce XSS or something like that.
• the password change is not implemented at all.
• make sure I didn't mess up any of the login providers along the way (i.e. disable password boxes for ldap or github ...)

-----8<---- MVP line, cut here if neccessary ---8<----

• (rudimentary) visual styling (right now, this is intended for code critique and conceptual work. Styling will be done later, of course.)
• translations should be added to the correct jsons
• username + displayname (display name defaulting to username, username defaulting to whatever it would have been so far. Perhaps the profile json will no longer be neccessary)
• have /user/:username actually use username instead of user.id
• documents i contributed and document I own is a bit tricky, those should be opt-in since they make documents discoverable that would otherwise have remained hidden. (checked off for "won't be done for now")

For a MVP we could "just not link to it, yet" but already deploy it. We could get some feedback this way and improve it more before we officially ship it.

[SISheogorath]

We could put "delete user" on that page instead of the index-page-menu. Opinions?

Definitely! It was only put into the menu on the main page, because there was no profile page yet.

Did I get all the places, where those variables are used? Does EJS have a way to detect used but unset variables (like a strict mode for EJS, perhaps)?

Not sure if you got all, but EJS will definitely complain when there is something missing.

---

All in all, I really like your work! Seems like user profiles are moving forward! And I'm really curious about what we will get as result!

[davidmehren]

How does this interact with users from other auth sources, eg. LDAP? Normal apps might not have permission to change user attributes or delete them.

[SISheogorath]

@davidmehren right now, we delete all user data that exist in CodiMD's database when someone requests an account deletion. This means they can definitely re-auth with their external credentials afterwards, but they start from scratch, like their account never existed before.

Just realized you referred to the editing of the attributes in CodiMD. Not sure if we want to restrict them. How do other application handle that? I think most of them simply allow to change these details. At least discourse and GitLab do so, as far as I can say.

[ccoenen]

Thanks for reminding me, until now i was only concerned with local users. I will add it to the TODO list. Since I use none of the other login providers, I'd value any input on that part. I might need to disable password change for those users, but display name (name attribute) ist completely local in any case. It's what will be listed in the "2 ONLINE" button top right, for example.

[ccoenen]

this is just an example. It should be replaced before this gets merged. What do we want to allow for display names? Personally, I wouldn't want to restrict this too much.

[SISheogorath]

Mhm, should we make a difference between a Display name and a username? I think that would be useful.

so johndoe is my username (where I would go for something like [a-z0-9_-]) and a Display name: John Smith which is free because as display name people may want to use non-latin characters since their Name is Arabic, Chinese, German, French, …

[ccoenen]

the name attribute is meant as display name. I'll change the property (and db field) name to reflect that. But even with that in place, I'd like to disallow at least <>'"=%/:

currently, we don't have a username. For local users, the email address would be the closest thing. For remotely authenticated, I have no clue what would be in its stead.

I wouldn't want to create a third thing (email - username - displayname) as we have not technical neccessity for it. We could add something like a shorthand. Like John Doe -> johnd that could (one day) be used for things like @johnd (which I would like to link back to the user profile) or in urls (think /profile/johnd or perhaps /johnd/note-1).

[ccoenen]

we could of course introduce (technical) username and (human readable) display name and

• make email optional (for now, we're not using or verifying it anyway)
• make display name default to username, so people don't have to provide five thins when signing up.

[SISheogorath]

Yes, that should be the way to go. We already have the concept of display names in various (maybe all except local?) login methods, so I think we should do this.

And while the username can be quite strict, the Display name allows all kinds of versions. And of course, yes, the default display name should simply be the username.

[SISheogorath]

I would even go for 3 characters as minimum length

[ccoenen]

this change is a by-product of a validation test I made. I shouldn't have committed it. Personally, I see no real reason to restrict this at all (beyond "must be set to something printable"). In chinese or japanese, a single character could be a proper name. I did set an upper bound, because the database field is a varchar(255), though.

[ccoenen]

In two places I did not conform to the normal "human readable" pattern of this file. This is the first: the validation error messages from sequelize are just bad. To know which field is affected, I concatenate them with a slash.

[ccoenen]

this is the other place: I want to translate the user properties, and I think it is important that we know that this is about the user model.

[ccoenen]

I will be going for username / displayname next. Along the way I'll probably rebase this a few times, so previous discussion might end up folded up in github.

[ccoenen]

closing in favor of hedgedoc/hedgedoc#34

[Choque]

Just wondering, was the profile page ever been implemented in any way? I just spun up a CodiMD instance in docker and I can't seem to find my profile page nor change the (auto-generated) username. At least changing the username and profile picture would be something I'd really like to do.

[ccoenen]

I no longer contribute to CodiMD, but I proposed some of this in hedgedoc/hedgedoc#34 as mentioned above.