Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented login page. #2

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Implemented login page. #2

wants to merge 7 commits into from

Conversation

rottencandy
Copy link

Created a login page in mithril.js as part of the task 101 of the project.

login.js when used as a module works as a mithril component that renders a
login page and communicates with RS Json API to faciliate validation.
Also updated qmake file to include login.js in build.
Added extra function to verify API keys(to be used by login.js) and
updated HTTP requests function to use a different callback on failiure.
index.html's initial startpoint now loads all pages using mithril's
route functionality.
And changed downloads.js to disable updating of downloads list before the
keys have been verified.
Communication with Json API now only happens if the keys are verified.
Automatic rendering and reloading of downloads is handled by oninit attribute
of the downloads component.
@G10h4ck
Copy link

G10h4ck commented Apr 6, 2019

so from my tests the login system you implemented seems to work, but probably involuntarily the download page is now broken and the download file list is never requested, about mithril update, I think we should not include it minified in our repository, it is impossible to review WTF has been really included, and if we need minified it should be done at compile time, not included in our repository, as it would pose serious security concerns about the webui...

@G10h4ck
Copy link

G10h4ck commented Apr 6, 2019

So I have been looking at the differences between mithril.js minified and not minified, for such a minimalist framework I think using the minified source is not worth the effort/risk, so we should remove the minified version completely, and make sure the RS webui keeps working

@rottencandy
Copy link
Author

Yes, the minified version is present, but it was not being used in the build process. I will look into the downloads page.

But I think debugging is still difficult, because what qmake essentially does is, take all the .js scripts and append them to a single file. Makes it hard to figure out from which script the error occurs.

Due to security and debugging issues, the minified version will not
be used in the build process anymore. The framework is pretty minimalistic
so it would not make that big of a difference.
Previously, the downloads page would be loaded but API would not
be accessed. This was causing issues with mithril's auto rendering.
Not loading the downloads component will make sure no requests are
being sent before login finishes.
@rottencandy
Copy link
Author

Removed minified script.
I think the problem with downloads component was that it sends requests to the API server as soon as it is created (before login verification, thus failing). Now modified and should be created only after the login credentials have been verified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants