| [20:42:17] | * paul121[m] has quit (Ping timeout: 246 seconds) |
| [20:42:24] | * paul121[m] has joined #farmos |
| [20:42:39] | <symbioquine[m]> | paul121: I think I blew away that environment since then, but it shouldn't be hard to retest. It sounds like you're describing the opposite problem I was encountering. |
| [20:43:28] | <symbioquine[m]> | I was observing that the default `farm` oauth client could be used for an Authorization Code grant with ***any*** redirect url. |
| [20:51:04] | <symbioquine[m]> | or perhaps I'm not reading that issue correctly...? |
| [20:53:00] | <paul121[m]> | Yeah you're right. It is different. I tried it in a fresh environment and the issue I linked to above doesn't happen |
| [20:53:42] | <paul121[m]> | Pinning down the jwt library at 3.3.3 prevents league/oauth2-server 8.2 from installing, which is where that bug was introduced |
| [20:54:32] | <paul121[m]> | Makes me wonder if we should run some of the simple_oauth tests inside our tests. Just because these things once get noticed as frequently in their repo unless people are running the tests :-/ |
| [20:55:32] | <paul121[m]> | * Makes me wonder if we should run some of the simple_oauth tests inside our tests. Just because these things wont get noticed as frequently in their repo unless people are running the tests :-/ |
| [20:57:27] | <paul121[m]> | If farmOS were installed with league/oauth2-server v8.2 it would have broken the oauth functionality for clients w/o a redirect uri |
| [20:59:58] | <symbioquine[m]> | Broken would be a much better outcome in this case. |
| [21:00:34] | <paul121[m]> | I guess this is an issue for any dependency of a dependency that releases breaking changes when they shouldn't 😅 |
| [21:01:26] | <symbioquine[m]> | It sounds like that might be the case with the issue you were mentioning. I'm not sure we have enough information to say what the cause of the one I described is... |
| [21:02:43] | <paul121[m]> | Yep I agree!! That behavior seems to have been there for a while |
| [22:17:58] | <symbioquine[m]> | Just reproduced the behavior again. The relevant versions are `league/oauth2-server=8.1.1`, `drupal/simple_oauth=5.0.2`, `lcobucci/jwt=3.3.3` |
| [22:18:40] | <symbioquine[m]> | ACTION posted a file: composer.lock (367KiB) < https://matrix.org/_matrix/media/r0/download/matrix.org/JrcEfotunzEbsIfL... > |
| [22:20:36] | <symbioquine[m]> | The "test case" I'm using is; do a standard installation with the dev docker image, log in, then paste this into the url bar "http://localhost/oauth/authorize?response_type=code&client_id=farm&redir... |
| [22:22:35] | <symbioquine[m]> | If the test case "fails" it will have redirected to http://some.random.domain/?code=some-really-long-hex-string-here |
| [22:24:22] | <symbioquine[m]> | I believe the correct behavior would be for an error to be returned saying the Authorization Code grant flow is not supported for the `farm` client id, or an error saying that the domain `some.random.domain` is not allowed to request the Authorization Code grant flow. |
| [22:25:50] | <symbioquine[m]> | In the failing state, I believe it would represent a pretty big security issue. |
| [22:29:57] | <paul121[m]> | oh yes! sorry I thought you were referring to the 401 instead of 400 error |
| [22:30:47] | <symbioquine[m]> | Yeah, sorry if that was confusing. I've discovered at least two issues with oauth in farmOS 2.x at this point. :) |
| [22:31:54] | <symbioquine[m]> | I think the one associated with [this issue](https://github.com/thephpleague/oauth2-server/issues/1162) is already tracked in the right place. |
| [22:32:29] | <symbioquine[m]> | However, the one I just described above does not yet have a tracking issue that I know of. |
| [22:35:59] | <paul121[m]> | definitely! I forgot to test this today |
| [22:36:08] | <paul121[m]> | want to open an issue? |
| [22:36:31] | <symbioquine[m]> | Sure |
| [22:38:03] | <paul121[m]> | Just tried it really quick and am seeing the same |
| [22:38:10] | <paul121[m]> | but don't have much time to dig in |
| [22:38:35] | <paul121[m]> | does make me curious if it is related to the "requiring a redirect_uri" issue, though |
| [22:39:22] | <symbioquine[m]> | Maybe, but it's a really bad failure mode in that case :) |
| [22:39:41] | <paul121[m]> | there was a bug when an empty array was returned, so they made the requirement to add a redirect_uri.. but this behavior wasn't mentioned |
| [22:39:58] | <paul121[m]> | yeah haha |
| [22:40:33] | <paul121[m]> | if I set a redirect URI I see that the Authorization Code auth to a "random" domain fails, is that the same for you? |
| [22:44:31] | <symbioquine[m]> | Yeah, it gives an error "Fatal error. Unable to get the authorization server." |
| [22:45:13] | <symbioquine[m]> | But I believe having a domain set does not yield the desired behavior for the resource owner grant flow right? |
| [22:47:51] | <symbioquine[m]> | There's also this UI bug in simple_oauth; |
| [22:47:56] | <symbioquine[m]> | ACTION uploaded an image: image.png (48KiB) < https://matrix.org/_matrix/media/r0/download/matrix.org/mBCkELZxRzGnMniW... > |
| [22:49:53] | <paul121[m]> | Hmm I'm not sure I've tried that. But I think it should work with a redirect uri configured, even if it isn't necessary? |
| [22:50:06] | <paul121[m]> | Good question tho |
| [22:50:42] | <paul121[m]> | That issue is actually documented in their issues! |
| [22:51:06] | <paul121[m]> | Well.. documented isn't correct |
| [22:51:18] | <paul121[m]> | There's an open issue for that :-) |
| [22:55:31] | <symbioquine[m]> | Opened https://www.drupal.org/project/farm/issues/3185721 |
| [22:57:27] | <symbioquine[m]> | I just tested and the Resource Owner grant flow actually does still work even with the redirect uri constrained to "http://example.com/". |
| [23:18:29] | <paul121[m]> | Hmm yeah I'm not sure what expected behavior would be for that. I think in the oauth2_server module for D7 had it configurable if the redirect_uri needed to match for the resource owner/password flow |
| [23:21:44] | <symbioquine[m]> | the oauth2_server module also allowed per-client-id whitelisting of which grant flows were allowed. |
| [23:22:41] | <symbioquine[m]> | So it would have been possible to only allow the resource owner flow. |
| [23:23:00] | <symbioquine[m]> | * the oauth2_server module also allowed per-client-id whitelisting of which grant flows were permitted. |
| [23:25:02] | <symbioquine[m]> | mstenta: You might find [this](https://github.com/symbioquine/farmOS_wfs/blob/2.x/.github/workflows/run...) interesting. It makes the browser output from the tests a downloadable artifact; e.g. https://github.com/symbioquine/farmOS_wfs/actions/runs/392366402 |
| [23:31:07] | <symbioquine[m]> | <symbioquine[m] "the oauth2_server module also al"> It also seems to have the sane behavior where no whitelisted redirect uris for a client id means it doesn't allow any flows which require redirection - for that client id. |
| [23:44:12] | <symbioquine[m]> | <symbioquine[m] "image.png"> Here's the issue for the UI bug; https://www.drupal.org/project/simple_oauth/issues/3116782 |
| [07:07:36] | <mstenta[m]> | Wow just catching up on all this |
| [07:07:49] | <mstenta[m]> | TL;DR? |
| [09:55:58] | <paul121[m]> | Need to look into the issue symbioquine opened |
| [09:55:58] | <paul121[m]> | > Opened https://www.drupal.org/project/farm/issues/3185721 |
| [09:56:57] | <paul121[m]> | we could patch that UI bug as well. Its been open for a long time, I was hoping it would get merged. maybe we just need to bump up the issue |
| [10:36:01] | <paul121[m]> | ACTION uploaded an image: 99013315-0b533a00-2505-11eb-8b4f-7685f18fba93.png (32KiB) < https://matrix.org/_matrix/media/r0/download/matrix.org/EgkPUnbyVdtAtfpm... > |
| [10:36:01] | <paul121[m]> | custom icons |
| [11:06:28] | <paul121[m]> | we were chatting about icons this morning |
| [11:07:23] | <paul121[m]> | mstenta mentioned this project that is going to develop royalty free food/agriculture icons: https://ourfood.world/foodicons/ |
| [11:07:53] | <paul121[m]> | maybe we can use them one day :D |
| [11:08:17] | <mstenta[m]> | I was thinking... maybe we should design and submit one specifically for a farmOS logo :-) |
| [11:08:36] | <mstenta[m]> | What would look good in a hexagon and represent farmOS? Ideas? |
| [11:08:51] | <mstenta[m]> | (thinking it would still be alongside the existing "farmOS" text logo) |
| [11:09:08] | <mstenta[m]> | (but could be used as a square-aspect ratio logo/icon in certain contexts) |
| [11:13:01] | <mstenta[m]> | As much of a cliche as it is, I feel like a leaf/seedling icon is the best all around fit for farmOS :-) |
| [11:13:26] | <mstenta[m]> | Tractor... barn... etc are other ideas, but feel too limiting. farmOS is more than that :-) |
| [11:13:53] | <mstenta[m]> | So maybe we can just adopt one of the icons they create as our own haha |
| [11:14:40] | <wombat83[m]> | <mstenta[m] "Tractor... barn... etc are other"> What if those things are in the corners of the hexagon with the farm OS logo at the center of the hexagon linked with spokes? |
| [11:14:52] | <mstenta[m]> | oh that's an idea |
| [11:15:21] | <mstenta[m]> | i think the FoodIcons design guidelines are relatively strict/simple... might be hard to fit something in when the icon is very small |
| [11:15:36] | <wombat83[m]> | I know it might be difficult to get the icon details into a small logo but it might be worth trying maybe putting icons of food or farming equipment into the corners of the hexagon |
| [11:15:40] | <mstenta[m]> | oh but... it makes me thing... |
| [11:15:49] | <mstenta[m]> | hexagons fit together well... |
| [11:16:15] | <mstenta[m]> | perhaps we create a splash image on the farmOS website with the farmOS hexagon in the middle and tractor, barn, etc ones around it :-) |
| [11:17:38] | <paul121[m]> | wow yes.... every module gets an icon & you build your hexagon puzzle |
| [11:18:06] | <mstenta[m]> | oooooh :-) |
| [11:18:13] | <paul121[m]> | we also need a fun 404 page |
| [11:18:49] | <mstenta[m]> | Ah cool I just checked the guidelines doc and it says Feb 14 is the deadline for the "final refinements phase", followed by an "approval phase" after that... so maybe these icons WILL be available before the release of farmOS 2.x :-) |
| [11:18:56] | <mstenta[m]> | hahaha yes. yes we do need a fun 404 page |
| [11:19:23] | <mstenta[m]> | i mean... maybe what we REALLY need is a mascot haha |
| [11:19:27] | <mstenta[m]> | like octocat |
| [11:19:36] | <mstenta[m]> | https://github.com/404 |
| [11:20:23] | <wombat83[m]> | Muscular field mouse? 🐁 |
| [11:20:44] | <mstenta[m]> | hahaha |
| [11:20:50] | <mstenta[m]> | oh man a mouse is a great idea haha |
| [11:21:16] | <mstenta[m]> | goat is also an option! https://goatech.org |
| [11:24:16] | <symbioquine[m]> | ...maybe the kinglet? |
| [11:24:49] | <symbioquine[m]> | https://en.wikipedia.org/wiki/Kinglet |
| [11:25:34] | <mstenta[m]> | ooh i like that too! |
| [11:26:50] | <symbioquine[m]> | ACTION uploaded an image: image.png (78KiB) < https://matrix.org/_matrix/media/r0/download/matrix.org/TUAuIELtIHpJnsea... > |
| [11:52:32] | <symbioquine[m]> | mstenta: What do you think about copying [these lines](https://github.com/farmOS/farmOS/blob/2bce16f9fc87d9d6219603857aef2a1316...) into the prod Dockerfile? It would make it easier to build a "user test-runner" image with the uid/gid set to match the dev one... |
| [11:52:50] | <symbioquine[m]> | Obviously with the default value set to match the current default of 33. |
| [11:53:35] | <mstenta[m]> | Oh sure I don't see why not? |
| [11:54:03] | <symbioquine[m]> | Cool :) |
| [11:57:17] | <mstenta[m]> | Curious how we'd use that in practice specifically? |
| [11:59:47] | <mstenta[m]> | We'd be able to move the `--build-arg WWW_DATA_ID=33` flag in `run-tests.yml` to the `farmos/farmos:2.x` image build... but that's pretty negligible. |
| [12:00:37] | <mstenta[m]> | In the context of contrib modules... ideally we don't need to build the image at all, because that makes the action take longer to run. |
| [12:00:49] | <mstenta[m]> | So I don't think we'd be able to use it in that context |
| [12:01:11] | <mstenta[m]> | * So I don't think we'd necessarily want to use it in that context |
| [12:01:32] | <mstenta[m]> | Are you thinking for local testing? |
| [15:46:40] | <mstenta[m]> | paul121: how did you go about customizing the icons in your screenshot above? |
| [15:47:16] | <mstenta[m]> | i'm working on the 2.x menu right now - thinking about swapping out the Drupal icon for the farmOS logo as a first step |
| [15:47:51] | <mstenta[m]> | (is it just using the "logo" setting in the theme?) |
| [15:52:19] | <mstenta[m]> | Answer: yes it is just the "logo" setting :-) |
| [15:52:57] | <paul121[m]> | yup! |
| [15:54:26] | <paul121[m]> | the icons was just using css to set the `:before` background-image |
| [15:55:22] | <paul121[m]> | I know theres that whole deal about matching the color type, but you can just change the color of the icon if you're providing your own SVG |
| [15:56:02] | <paul121[m]> | rather than how Drupal has the same SVG saved in different colors (which must be the better way to go about it?) |
| [15:56:20] | <paul121[m]> | ACTION sent a long message: < https://matrix.org/_matrix/media/r0/download/matrix.org/QVUCifFUYSKXaKFn... > |
| [16:34:19] | <paul121[m]> | symbioquine good find on the redirect bug.. even though there is an easy fix, it seems pretty important.. especially when `simple_oauth` provides a default client (just like we were doing). I made a test that recreates the behavior in simple_oauth: https://www.drupal.org/project/simple_oauth/issues/3185909 |
