News feed

[cedricbonhomme]

WIP chg: [feed] It is now possible to specify if the new feed must be returned to the RSS or ATOM format with a parameter in the URL Possibility to get RSS/ATOM news from all sources. Added HTML link tags in the header of main.html template.

[Rafiot]

Thank you!

[adulau]

A small note or discussion for the formatting, should we agree on a black configuration to run it in the CI to ensure always the same formatting? @cedricbonhomme @Rafiot what do you think?

[cedricbonhomme]

It's OK for me, we can do that.

I even often do this client side with pre-commit. For example:

• https://github.com/cedricbonhomme/newspipe/blob/master/.pre-commit-config.yaml#L13-L16
• https://github.com/cedricbonhomme/newspipe/blob/master/pyproject.toml#L68-L69 to align isort with black

How you prefer for the format. We can dedicate a PR to this.

[Rafiot]

I'm fine with black, but not the line length : this is genuinely less readable most of the time.

The single/double quote thing, I don't think I care - as long as it doesn't forces me to do weird escapes, is in a dedicated PR, and I can enforce it in pre-commit on my side. And I think that's the only things you're doing different than me (?)

My vimrc config for for flake8 is that:

let g:syntastic_python_checkers=['flake8']                                                                                                                                                                                                                                                                                  
let g:syntastic_python_flake8_args='--ignore=E501,E252,W503'

[cedricbonhomme]

Actually I do not really care about line size. Depending of the project I uses limits around 120 or 140 characters, I would say... Currently it seems we have rarely a line with more than 125 characters.

Fine for flake 8 as well.

If you want I can make a dedicated PR for this and we can adjust the settings (flake, black).

[Rafiot]

The problem with black is (or at least was) that is doesn't only cut a long line, it also tries to fill as many characters as possible so it goes to whatever the limit is. And that's often even worse :/

Is there a way to tell black to just ignore that completely, without giving it a length?

[cedricbonhomme]

I do not think that black is doing that. It is wrapping lines based on various rules. When you have a long list of arguments for a function. Or when you have conditions, in their example:

if (
    some_long_rule1
    and some_long_rule2
):
    ...

Concerning list/set with a certain number of elements the result will be this, one element per line. If you have few elements in the list/set, it will depend if there is a coma after the last element. If no coma, black won't do that.

Now if you compare these 2 files:

• https://github.com/cve-search/vulnerability-lookup/blob/main/website/web/__init__.py#L54
• https://github.com/cedricbonhomme/vulnerability-lookup/blob/test-code-style/website/web/__init__.py#L53

and these 2 files:

• https://github.com/cve-search/vulnerability-lookup/blob/main/vulnerabilitylookup/vulnerabilitylookup.py#L41
• https://github.com/cedricbonhomme/vulnerability-lookup/blob/test-code-style/vulnerabilitylookup/vulnerabilitylookup.py#L41

You will see that there is few line cuts. The only thing I did is increasing the default limit of 85 to 140. I also do not really like 80 or 85 as a limit. Generally I am naturally close to a limit of 125.

Concerning double quotes, their rationale is here. I admit I agree. I found a way to disable it: --skip-string-normalization. They do not recommend to use it, unless in order to ease the adoption of black on existing projects.

The configuration for pre-commit that I used is here. We can adjust various things if you want.

[Rafiot]

Thank you for the references, I'll give it a shot. The quote thing, I'm cool with it, I just don't want to have to think about it (which is solved by the pre-commit config, so all good). I'll make a commit to change all that at once and then have it by default.

For the line length, I'll check, because last time I played with black, it reformated my code in a way I really hated (and that's the reason I don't use it). But that was years ago so they probably improved. Depending on the context and what the long clauses do and mean, there are often good reason to group them on one ligne/split on different lines that cannot really be automated.

To go back to this PR, should I merge it? Or do you want to change more things?

[cedricbonhomme]

Yes, you can merge it! We can improve more later. For example having the JSON of the vulnerabilities in content of the feed's items (for all sources).

For now, I think it is more useful that I start to work on authentication. I have looked a bit in Pandora (and Lookyloo). If you want, I can do like it is working in Pandora, storing the users in the kvrocks storage (self.storage, etc.).

And if you want I can open a PR for the style change. No need to merge it now.

[Rafiot]

Authentication in lookyloo and pandora is super specific, I'm not sure we can do the same in vulnerability lookup.

For lookyloo, it is just an admin user, for pandora it is an admin user and lightweight sessions. What would an authenticated user need to do in vulnerability lookup? I'm happy to let you do that, I just want to avoid we do it multiple times when new requirements come up.

[cedricbonhomme]

Thanks for the merge !

Yes, I agree that it is quite specific. I was mainly referring to the fact of using the same database to store the vulnerabilities and the (new) informations related to the users. So kvrocks. Most of the time I deal with PostrgreSQL (with SQLAlchemy). I am very familiar with Flask-Login, Flask-WTF, etc. But I suppose that in this project we will as well use kvrocks in order to store users ? Maybe I am wrong.

I was as well wondering why do we need authenticated views in vulnerability-lookup. @adulau gave us the example of users able to comment a vulnerability. So it is not even an admin user, but a normal user able to add comments. Of course, I guess we will need at some point an admin user. That's not really the difficulty.

[Rafiot]

Admin user is easy, and the session thing is fine too for what it is. The admin users for lookyloo and pandora are hardcoded in the config, the sessions are expired automatically after a while.

Adding a proper user management system (also encoding vulns potentially, see https://github.com/cve-search/vulnerability-lookup/issues/1#issuecomment-1735110315) is a completely other beast (especially when we will want to add groups, organisations, password reset, ...). Would be nice to scope it so we don't have to redo it in 6 months when we realize it doesn't scales.

[cedricbonhomme]

OK, I see we have a few open questions on this point ;-)

What I already did in at least 2-3 projects with Flask is having two kind of users: admin and normal users. With a support of organizations. A user can be member of one or several organizations. Implemented password reset, API key, etc. With the notion of owner on different kind of objects: owner as member of an org or as an object 'editor'. So yeah, it must be scoped/specified in a dedicated issue.

I am just still wondering about the backend where to store this. In kvorcks or we use Postgre/MarisDB (with SQLAlchemy), That's my main big question for the moment since I have some experience with what I explained above. I think that using a dedicated database it not an issue in order to implement a vulnerabilities encoding feature. Of course we have to talk about rights/permissions issues (object editor/contributor/etc.).

[cedricbonhomme]

Just created #32 to discuss. It's open of course.