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

Auth improvements #476

Closed
wants to merge 3 commits into from
Closed

Auth improvements #476

wants to merge 3 commits into from

Conversation

erno
Copy link
Member

@erno erno commented May 22, 2012

  • Async-enable the authentication hook
  • Expose QNetworkAccessManager in asset api so can enable http authentication there

Only thing is that asset api seems to be accessible also from untrusted scripts.
It already has some functionality that shouldn't be exposed (eg. like ability
to upload arbitrary stuff from filesystem to http storages). This should be solved
by either hiding the asset api from untrusted scripts, moving the sensitive
slots out of AssetAPI.

erno added 3 commits May 22, 2012 12:04
authentication with assets.  Also add untested and unfinished test script to
exercise it.
(through exposing the QNetworkAccessManager).
@erno
Copy link
Member Author

erno commented May 22, 2012

collecting feedback from irc:
12:41 < Pforce_> e`: virtual QNetworkAccessManager* GetNetworkAccessManager() {
return NULL; }
12:42 < Pforce_> 0 would be nicer, we are not a c app

12:45 < Stinkfist> e`: you can remove GetNetworkAccessManager from IAssetStorage
12:46 < Stinkfist> put to HttpAssetStorage only
12:46 < Stinkfist> goes to same metaobject anyways, so can be accessed ok from
scripts
12:46 < Stinkfist> also remove the Get-prefix ;)

@juj
Copy link
Contributor

juj commented May 28, 2012

Having a virtual QNetworkAccessManager* IAssetStorage::GetNetworkAccessManager() is not ok, as it leaks implementation details only relevant to HttpAssetStorage to the IAssetStorage implementation. If this is for scripting purposes, the property can be exposed at runtime without exposing the details into the base class, as Stinkfist outlined.

@juj juj closed this Jun 19, 2012
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