Add iCal link #146

frereit · 2022-02-07T19:15:01Z

An ical link is now available on /calendar.ics.
It contains all the ctfs with their description and dates.
This endpoint is protected with a token by default,
the password can be changed/removed by the admin.
#116

This work is mostly by @B-i-t-K from PR #116 but I finished it by adding a /calendar.ics route in the nginx proxy and slightly updating the frontend.

Since both graphql.schema.json and schema.ts are deleted, shouldn't we add both to the gitignore ?
#116 (comment)

I added back graphql.schema.json because it should be deleted in a seperate PR / merge or directly on dev to avoid doing multiple things on one branch.

I see the pool is only used for the ical route.
Can we use this for postgraphile too ? It would be nice to share the same connection.
#116 (comment)

Yes. This is done now, and the pool uses the postgraphile "user" instead of "admin"

Edit:
And while this PR is open: We also made some more changes to CTFNote:

Fix the table view to wrap lines instead of overflowing the window border.
Add a "last active date" to the admin backend for each user
Add authentication with hedgedoc so that /pad/ isn't publicly-writeable (and writeups are secret by default)
Add better password requirements by integrating with the haveibeenpwned API.

If you want any of these changes upstream just let me know and I can open a PR for any one of those.

An ical link is now available on /calendar.ics. It contains all the ctfs with their description and dates. This endpoint is protected with a token by default, the password can be changed/removed by the admin.

gen_random_bytes is more suited for crypto purpose

…ream-dev

If it should be removed, this should be done in a seperate PR.

Adds /calendar.ics route to frontend Fixes changing the ical key slight frontend changes when displaying the ical link

api/src/routes/ical.ts

front/src/components/CTF/Calendar.vue

This reduced code redundancy

JJ-8

The code looks fine to me. Some small things only have to be changed in order to merge this.

About the other PRs:

Fix the table view to wrap lines instead of overflowing the window border.

Is this related to / based on #111? And is it similar?

Add a "last active date" to the admin backend for each user

I like the addition. Could be useful when cleanup the database.

Add authentication with hedgedoc so that /pad/ isn't publicly-writeable (and writeups are secret by default)

I think this was based on my code of CTFNote 1.0? We had a discussion in #68 about this and because it was quite a hacky solution, it was decided to not add this. Also, Hedgedoc 2.0 is coming up and that would definitely break this integration so I don't think that it will be merged for now...

Add better password requirements by integrating with the haveibeenpwned API.

Sure, why not? If you already have the code and it works, I would be happy to merge it!

front/src/components/Admin/Registration.vue

front/src/ctfnote/settings.ts

front/src/components/Admin/Registration.vue

frereit · 2022-02-11T15:10:07Z

Thank you for the review. Changes should be added later today, but it might also take a few days because I'm away for the weekend.

Is this related to / based on #111? And is it similar?

Yes, I think it's pretty much the same thing, no need to merge.

it was quite a hacky solution, it was decided to not add this.

I agree that it's a hacky solution. Hedgedoc 2 should fix it but we didn't want to wait.

I will open the other two PRs next week :)

* Fix eslint errors * Fix insecure randomness in Registration.vue

JJ-8

Thank you for your contribution!

@XeR, can you review this too and merge this?

XeR

Thanks for your work on this feature. Sorry for the slow response time.

On top of the comments regarding your changes, I noticed you changed the destination URL.
On @B-i-t-K's branch, the iCal URL points to the CTF platform, while on yours it points to CTFNote.
I have mixed feelings about this. I'd like to have users' input.

My personal use case is to have icalendars synchronized with a smartphone.
I never connect to CTFNote from a smartphone.
I would rather have the link point to the CTF platform so that I can see how my team is doing on the scoreboard.
I used to import ics files from CTFtime to a Nextcloud instance.

What are your use cases ? Do you synchronise this with a software calendar on a computer (Thunderbird, Apple Calendar, Microsoft Whatever, etc.) ?

api/src/routes/ical.ts

api/package.json

XeR · 2022-03-01T22:16:49Z

api/src/routes/ical.ts

+    for (const ctf of ctfs) {
+      // Assumes that CTFNote is hosted at the root of the domain
+      const ctf_url = new URL(
+        `/#/ctf/${ctf.id}-${slugify(ctf.title)}/info`,


See comment about the slugify package.
Can we retrieve ctf.slug instead ? See /front/src/components/Utils/CtfNoteLink.vue, line 29.

we do not use this package

CTFNote/front/src/ctfnote/ctfs.ts

Lines 47 to 49 in c125a07

function safeSlugify(str: string) {

return slugify(str) || 'no-slug-for-you';

}

The ctf.slug is generated in the frontend when creating a Ctf from a CtfFragment, so it's not available to the API:

CTFNote/front/src/ctfnote/ctfs.ts

Lines 89 to 91 in c125a07

export function buildCtf(ctf: CtfFragment): Ctf {

const slug = safeSlugify(ctf.title);

const params = { ctfId: ctf.id, ctfSlug: slug };

I don't really see why the API shouldn't just generate the slug itself, using the same function as the frontend. Especially because the slug doesn't matter, it's purely visual (as long as the id is correct, the CTF will show).

I think it is perfectly to regenerate it in the backend since it is nice to see which CTF you are going to visit when clicking the link. Do you agree @XeR?

So how should I approach / fix / resolve this?

I think you can leave it like this or if it is purely visual then you may also remove it.

@frereit, can you confirm that the slugify in the backend is purely visual? So that means that if we remove the slugify part from the URL, the link still works? If so, I think it is preferred to remove the dependency and therefore the slugified part of the URL. So we only show a URL with an ID in it. I don't think this is a problem since the event will already contain the name of the event.

Yes, the slug is purely visual, however just the id is not enough, so a valid URL for CTF with id 20 needs to match/\/#\/ctf\/20-.+\//. Just /#/ctf/20/ wouldn't work. I haven't looked into this, but should be trivial to change the logic to allow id-only links. If you want, I can do that.

I think that is the best solution. If you could change it to accept only IDs, that would be very nice!

frereit · 2022-03-02T07:27:30Z

What are your use cases ? Do you synchronise this with a software calendar on a computer (Thunderbird, Apple Calendar, Microsoft Whatever, etc.)

Yes, I synchronise this with Outlook. Regarding the problem with determining the URL of the CTF on CTFNote reliably, I think it might be best to point to the CTF itself. I could always add an X-CTFNOTE-URL parameter to the VEVENT that tries to point to CTFNote.

Note: Users can still send requests which yield garbage responses, but this is not a security vulnerability because these responses are only served to the requesting user

JJ-8 · 2022-04-09T13:06:19Z

front/src/router/routes.ts

@@ -54,6 +55,7 @@ const ctfRoute: RouteRecordRaw = {
  children: [
    {
      path: 'task/:taskId(\\d+)-:taskSlug',
+      alias: ['task/:taskId(\\d+)'],


When I am visiting a URL like http://localhost:8088/#/ctf/1/info and then click on the tasks tab, then I get an empty screen and the following warning in the console:

The URL that has the empty page is http://localhost:8088/#/ctf/1-Midnight-Sun-CTF-2022-Quals/tasks in my case. So this is probably due to the slug being added again to the URL.

Huh, interesting bug. I don't think the warning has anything to do with the bug, though (at least I can't reproduce the warning on my side).
Interestingly when just refreshing website at the same URL it works as expected. I am investigating, but no clue what's causing this yet. Adding an alias for a URL shouldn't break things this way imo.

Ok so I can't find an easy fix for this right now. Unfortunately I will be pretty busy again this week and might only get to this next week, sorry.

Implements TFNS#146 (comment)

BitK and others added 11 commits November 30, 2021 02:28

update git ignore

347da4c

remove graphql.schema.json from git (dev dep)

745b553

Added ical link

a57d749

An ical link is now available on /calendar.ics. It contains all the ctfs with their description and dates. This endpoint is protected with a token by default, the password can be changed/removed by the admin.

change random to gen_random_bytes

03065b4

gen_random_bytes is more suited for crypto purpose

Merge branch 'ical-link' of https://github.com/TFNS/CTFNote into upst…

b97d1f6

…ream-dev

Update number of ical link migration

9eaa2f7

Fix outdated wrapNotify call

950e01f

Add back graphql schema.

58f2de2

If it should be removed, this should be done in a seperate PR.

Add iCal Link

5280b8c

Adds /calendar.ics route to frontend Fixes changing the ical key slight frontend changes when displaying the ical link

Add option to disable iCal password

6ecb4fc

eslint fixes

1e40eff

frereit commented Feb 7, 2022

View reviewed changes

api/src/routes/ical.ts Outdated Show resolved Hide resolved

frereit commented Feb 7, 2022

View reviewed changes

front/src/components/CTF/Calendar.vue Outdated Show resolved Hide resolved

frereit marked this pull request as draft February 8, 2022 07:22

frereit marked this pull request as ready for review February 8, 2022 16:16

frereit added 2 commits February 8, 2022 17:17

Add missing dependencies

9799e0f

Change pool to use getDbUrl

123fcdd

This reduced code redundancy

JJ-8 requested changes Feb 11, 2022

View reviewed changes

front/src/components/Admin/Registration.vue Outdated Show resolved Hide resolved

front/src/ctfnote/settings.ts Outdated Show resolved Hide resolved

front/src/ctfnote/settings.ts Outdated Show resolved Hide resolved

JJ-8 reviewed Feb 11, 2022

View reviewed changes

front/src/components/Admin/Registration.vue Outdated Show resolved Hide resolved

JJ-8 mentioned this pull request Feb 11, 2022

Add word wrapping to table layout #148

Merged

Minor fixes

63f174f

* Fix eslint errors * Fix insecure randomness in Registration.vue

frereit requested a review from JJ-8 February 14, 2022 17:51

JJ-8 approved these changes Feb 15, 2022

View reviewed changes

XeR self-assigned this Mar 1, 2022

XeR requested changes Mar 1, 2022

View reviewed changes

JJ-8 requested a review from XeR March 30, 2022 10:32

frereit added 2 commits April 5, 2022 14:32

Fix crash when host is invalid.

a6cd463

Note: Users can still send requests which yield garbage responses, but this is not a security vulnerability because these responses are only served to the requesting user

Add /ctf/:ctfId route as an alias

55c35c0

frereit added 2 commits April 8, 2022 09:09

Make calendar links by id-only.

b2249c1

Merge branch 'dev' of github.com:TFNS/CTFNote into upstream-ical-link

373e473

frereit requested a review from JJ-8 April 8, 2022 11:17

Make taskSlug optional

f1e09b6

JJ-8 requested changes Apr 9, 2022

View reviewed changes

This was referenced Apr 25, 2022

Fix crash on invalid host header DU4L/CTFNote#61

Draft

Broken links in add-ical-url #163

Closed

broken link in add-ical-url DU4L/CTFNote#75

Open

XeR changed the base branch from dev to main August 24, 2022 18:49

JJ-8 added a commit to JJ-8/CTFNote that referenced this pull request May 5, 2023

Replace CTF link in ical with actual CTF URL

e4b4abb

Implements TFNS#146 (comment)

This was referenced May 5, 2023

0 upstream ical link fix JJ-8/CTFNote#23

Merged

Feature: iCal link #216

Closed

JJ-8 merged commit 2a2b816 into TFNS:main Apr 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add iCal link #146

Add iCal link #146

frereit commented Feb 7, 2022 •

edited

Loading

JJ-8 left a comment

frereit commented Feb 11, 2022

JJ-8 left a comment

XeR left a comment

XeR Mar 1, 2022

frereit Mar 2, 2022

JJ-8 Mar 16, 2022

frereit Mar 30, 2022

JJ-8 Mar 31, 2022

JJ-8 Apr 7, 2022

frereit Apr 7, 2022

JJ-8 Apr 8, 2022

frereit commented Mar 2, 2022

JJ-8 Apr 9, 2022

frereit Apr 12, 2022

frereit Apr 12, 2022

	function safeSlugify(str: string) {
	return slugify(str) \|\| 'no-slug-for-you';
	}

	export function buildCtf(ctf: CtfFragment): Ctf {
	const slug = safeSlugify(ctf.title);
	const params = { ctfId: ctf.id, ctfSlug: slug };

Add iCal link #146

Add iCal link #146

Conversation

frereit commented Feb 7, 2022 • edited Loading

JJ-8 left a comment

Choose a reason for hiding this comment

frereit commented Feb 11, 2022

JJ-8 left a comment

Choose a reason for hiding this comment

XeR left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frereit commented Mar 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frereit commented Feb 7, 2022 •

edited

Loading