CTFd / ctfcli

ctfcli is a tool to manage Capture The Flag events and challenges
https://ctfd.io/
Apache License 2.0
165 stars 67 forks source link

Refactor CTFCLI to more OOP approach #130

Closed MilyMilo closed 1 year ago

MilyMilo commented 1 year ago

So, this is a complete refactor of ctfcli as discussed. Notable changes:

Architecture

Functionality

Testing

Other

Breaking Changes:


Remaining TODOs:

When I was refactoring the code I've came-up with several other ideas for features which I'll create separate issues for.

pl4nty commented 1 year ago

Looking forward to seeing this merge, it'll simplify some CI/CD pipelines I'm hoping to release.

If you're open to changes, I had one request - could ctf install automatically perform a ctf sync if a challenge has a duplicate name? Currently I have to try ctf sync for each chal, then run ctf install if sync fails

MilyMilo commented 1 year ago

@pl4nty Sure it's possible and an easy change, but I'm just wondering if a desirable one.

While it's better from a CI/CD point of view where you can be sure you're just installing a challenge which might already be installed. However, we're mostly using this as a CLI - in which case it's most likely just a human error (either wrong command, or a wrong challenge - and we can't know for sure). We could end-up accidentally syncing challenges that are unfinished.

I think I'd see this as either a switch or a new command. e.g

ctf challenge install-or-sync or ctf challenge install --sync-if-exists

CC: @ColdHeat what do you think?

pl4nty commented 1 year ago

Thanks, a switch or new command would work well for my usecase. I'm currently grep-ing for the sync error to detect if install is required, which is pretty brittle (breaks on this PR)

ColdHeat commented 1 year ago

I like the idea of having install also do a sync if asked to do so. I think there's some other questions about where is the source of truth but I think a switch is enough. What about --force?

MilyMilo commented 1 year ago

@ColdHeat @pl4nty I actually think that doing a sync instead of installing the challenge as a duplicate is a better behaviour for --force. I'll adjust that.

pl4nty commented 1 year ago

Installing from git doesn't seem to be fixed, ie pip install git+https://github.com/CTFd/ctfcli. It was also failing before the branch was merged. Maybe this snipped is needed in pyproject.toml?

[tool.poetry.scripts]
ctf = "ctfcli.__main__:main"