rejectedsoftware / vibenews

Combined web forum and NNTP server implementation for stand-alone newsgroups
GNU Affero General Public License v3.0
44 stars 3 forks source link

auth tags broken? #32

Closed Laeeth closed 9 years ago

Laeeth commented 9 years ago

When I try to use the web interface to add myself to the 'general' auth tag group (specified when I created the group - I presume this creates the tag too if it does not already exist) I receive the following error:

500 - Internal Server Error

Internal Server Error

Internal error information:

object.Exception@../../../home/laeeth/.dub/packages/userman-0.2.4/source/userman/mongocontroller.d(157): The specified group name is unknown.

/usr/include/dlang/dmd/std/exception.d:405 pure @safe void std.exception.bailOut!(Exception).bailOut(immutable(char)[], ulong, const(char[])) [0x892cff] /usr/include/dlang/dmd/std/exception.d:363 pure @safe bool std.exception.enforce!(Exception, bool).enforce(bool, lazy const(char)[], immutable(char)[], ulong) [0x892c82] ../../../home/laeeth/.dub/packages/userman-0.2.4/source/userman/mongocontroller.d:157 userman.controller.Group userman.mongocontroller.MongoUserManController.getGroupByName(immutable(char)[]) [0x98dd35] source/vibenews/controller.d:134 userman.controller.Group vibenews.controller.Controller.getAuthGroupByName(immutable(char)[]) [0x9670f6] source/vibenews/admin.d:289 _D8vibenews5admin14AdminInterface10updateUserMFC4vibe4http6server17HTTPServerRequestC4vibe4http6server18HTTPServerResponseZ18T9lambda3TAyaZ9lambda3MFAyaZS7userman2id35T2IDTS7userman10controller5GroupZ2ID [0x96515c] /usr/include/dlang/dmd/std/algorithm/iteration.d:563 @property userman.id.ID!(userman.controller.Group).ID std.algorithm.iteration.T9MapResultS1318vibenews5admin14AdminInterface10updateUserMFC4vibe4http6server17HTTPServerRequestC4vibe4http6server18HTTPServerResponseZ9lambda3TAAyaZ.MapResult.front() [0x964df2] /usr/include/dlang/dmd/std/array.d:123 userman.id.ID!(userman.controller.Group).ID[] std.array.array!(std.algorithm.iteration.T9MapResultS1318vibenews5admin14AdminInterface10updateUserMFC4vibe4http6server17HTTPServerRequestC4vibe4http6server18HTTPServerResponseZ9lambda3TAAyaZ.MapResult).array(std.algorithm.iteration.T9MapResultS1318vibenews5admin14AdminInterface10updateUserMFC4vibe4http6server17HTTPServerRequestC4vibe4http6server18HTTPServerResponseZ9lambda3TAAyaZ.MapResult) [0x8f95b2] source/vibenews/admin.d:289 void vibenews.admin.AdminInterface.updateUser(vibe.http.server.HTTPServerRequest, vibe.http.server.HTTPServerResponse) [0x9617d7] ../../../../../home/laeeth/.dub/packages/vibe-d-0.7.25/source/vibe/http/router.d:270 _D4vibe4http6router9URLRouter13handleRequestMFC4vibe4http6server17HTTPServerRequestC4vibe4http6server18HTTPServerResponseZ21T9lambda3TmTAAyaZ9lambda3MFmMAAyaZv [0xa1e5f2] ../../../../../home/laeeth/.dub/packages/vibe-d-0.7.25/source/vibe/http/router.d:653 const(void function(immutable(char)[], scope void delegate(ulong, scope immutable(char)[][]))) vibe.http.router.MatchTree!(vibe.http.router.Route).MatchTree.doMatch [0xa1f931] ../../../../../home/laeeth/.dub/packages/vibe-d-0.7.25/source/vibe/http/router.d:586 void vibe.http.router.MatchTree!(vibe.http.router.Route).MatchTree.match(immutable(char)[], scope void delegate(ulong, scope immutable(char)[][])) [0xa1f1c0] ../../../../../home/laeeth/.dub/packages/vibe-d-0.7.25/source/vibe/http/router.d:263 void vibe.http.router.URLRouter.handleRequest(vibe.http.server.HTTPServerRequest, vibe.http.server.HTTPServerResponse) [0xa1e396] ../../../../../home/laeeth/.dub/packages/vibe-d-0.7.25/source/vibe/http/server.d:1711 bool vibe.http.server.handleRequest(vibe.core.stream.Stream, vibe.core.net.TCPConnection, vibe.http.server.HTTPListenInfo, ref vibe.http.server.HTTPServerSettings, ref bool) [0xa99d68] ../../../../../home/laeeth/.dub/packages/vibe-d-0.7.25/source/vibe/http/server.d:1482 void vibe.http.server.handleHTTPConnection(vibe.core.net.TCPConnection, vibe.http.server.HTTPListenInfo) [0xa98522] ../../../../../home/laeeth/.dub/packages/vibe-d-0.7.25/source/vibe/http/server.d:1376 void vibe.http.server.listenHTTPPlain(vibe.http.server.HTTPServerSettings).doListen(vibe.http.server.HTTPListenInfo, bool).lambda3(vibe.core.net.TCPConnection) [0xa97f45] ../../../../../home/laeeth/.dub/packages/vibe-d-0.7.25/source/vibe/core/drivers/libevent2_tcp.d:546 void vibe.core.drivers.libevent2_tcp.ClientTask.execute() [0xa7c61a] ../../../home/laeeth/.dub/packages/vibe-d-0.7.25/source/vibe/core/core.d:459 void vibe.core.core.makeTaskFuncInfo!(void delegate()).makeTaskFuncInfo(ref void delegate()).callDelegate(vibe.core.core.TaskFuncInfo*) [0x922c11] ../../../../../home/laeeth/.dub/packages/vibe-d-0.7.25/source/vibe/core/core.d:969 void vibe.core.core.CoreTask.run() [0x9d1524] ??:? void core.thread.Fiber.run() [0xb3c345] ??:? fiber_entryPoint [0xb3c0e7] ??:? [0xffffffff]

Laeeth commented 9 years ago

So I looked into it. I think it relates to a half-finished switch from authentication based on groups to using authtags (separate for rw and ro access). What seems to make most sense then is to store the tags as properties["authTags"]. Seems to work for me now, but I do not understand code base well enough to be sure. Let me know if it sounds interesting and I will tidy it up and make a pull request.

Other thing I added was smtp configuration for settings.json. I know you haven't implemented this functionality yet, but as with everything today the logic is the easy bit and its the scaffolding that's work.

s-ludwig commented 9 years ago

You are right, the switch was more the other way around, but that's pretty much it. I didn't use auth tags for a long time, so I didn't notice. This definitely is a broad hint ("Wink mit dem Zaunpfahl") that a high-level test suite is in order (if I just had time for that ;)).

The problem is that the full solution (fully switching over to using UserMan groups) requires some more effort, including a simple group management UI. So the property based fix sounds like the best short-term solution, given the constrained time resources. It just needs to be remembered that a possible later switch to groups will require an additional DB upgrade procedure - I'll open a separate ticket for that.

Is your properties change ready for a pull request, or should I quickly redo it locally?

Laeeth commented 9 years ago

Shall I send you pull request in next day or two, time permitting? I somehow broke the nntp (probably to do with group auth) and want to fix that before sending.

Next thing I was thinking of adding is ability to paste images using javascript clipboard API and upload attachments (for me - you may not want this - but others might).

Sent from my iPad

On 19 Oct 2015, at 09:40, Sönke Ludwig notifications@github.com wrote:

You are right, the switch was more the other way around, but that's pretty much it. I didn't use auth tags for a long time, so I didn't notice. This definitely is a broad hint ("Wink mit dem Zaunpfahl") that a high-level test suite is in order (if I just had time for that ;)).

The problem is that the full solution (fully switching over to using UserMan groups) requires some more effort, including a simple group management UI. So the property based fix sounds like the best short-term solution, given the constrained time resources. It just needs to be remembered that a possible later switch to groups will require an additional DB upgrade procedure - I'll open a separate ticket for that.

Is your properties change ready for a pull request, or should I quickly redo it locally?

— Reply to this email directly or view it on GitHub.

s-ludwig commented 9 years ago

That would be great! Attachment/image support definitely also sounds interesting. Ideally that would rely on multipart support in vibe.d for storing them, which is technically almost there, but it's currently buried in the form support code. I wanted to extract that into an own module, but didn't make it in time for 0.7.26.

Laeeth commented 9 years ago

Okay - will work on pull request. Goes more slowly as a bit sore and need to write code mesh talk. The attachment and image probably is too big a bite for me. But I hope to have a couple of people from D world help me on paid basis soon, and they can definitely do it well, and faster than me.

Are there docs on javascript generator btw (on different note)? I couldn't seem to find, but that may be my fault.

Sent from my iPad

On 20 Oct 2015, at 06:54, Sönke Ludwig notifications@github.com wrote:

That would be great! Attachment/image support definitely also sounds interesting. Ideally that would rely on multipart support in vibe.d for storing them, which is technically almost there, but it's currently buried in the form support code. I wanted to extract that into an own module, but didn't make it in time for 0.7.26.

— Reply to this email directly or view it on GitHub.

s-ludwig commented 9 years ago

I've decided to look into a solution that works with UserMan groups, but doesn't require a new management UI. It does so at the expense of leaving unused authorization groups in UserMan's database, so there is still room for improvement. It also requires to press the update button in the admin interface once for every existing group that has auth tags.

Laeeth commented 9 years ago

That's great, and I am sorry that I never finished what I started here - just had too much on my plate. Laeeth.

Sent from my iPad

On 5 Nov 2015, at 20:48, Sönke Ludwig notifications@github.com wrote:

I've decided to look into a solution that works with UserMan groups, but doesn't require a new management UI. It does so at the expense of leaving unused authorization groups in UserMan's database, so there is still room for improvement. It also requires to press the update button in the admin interface once for every existing group that has auth tags.

— Reply to this email directly or view it on GitHub.

s-ludwig commented 9 years ago

I was worried a bit that you might have already put more work in it that would get obsoleted by this, but I figured that you were just too busy. Without a readily available fix it was less work overall to directly go the groups route, so I went for that.