Format protected and public paths with site prefix

[billy34]

Update protected and public paths to include site prefix.
Issue #1203
Sorry just a proposition. I do have dev knowledge but nothing about rust.

[lovasoa]

This seems reasonable ! you can test it by running cargo run. I'm not sure we have a guarantee that public_paths will always start with the slash you expect

[billy34]

Yes I wrote it in my previous comment (on original PR).
We must take care of edge values.
As the protection of paths checks that current path does start with protected or public path then it is assumed that oidc_protected_paths and oidc_public_paths are kind of absolute paths
So ensuring that elements of these paths begin with "/" before prepending site_prefix seems legit to me.
Am I right ?

[lovasoa]

I don't think we should silently correct values, we should validate the presence of the initial / in https://github.com/sqlpage/SQLPage/blob/main/src/app_config.rs#L108

[billy34]

I looked at the errors reported by CI.
I think I reached my limits and I'm not accustomed to the notion of borrowing variables :-)
I don't want to weaken this code by my very limited knowledge of rust!
Should I follow the hints provided (add mut to the variable declaration) ?

        let mut protected_paths: Vec<String> = config.oidc_protected_paths.clone();
        let mut public_paths: Vec<String> = config.oidc_public_paths.clone();

[lovasoa]

Instead of cloning the strings as-is and then mutating them, you can create the correct strings with format! directly the first time !

[billy34]

I agree. Looks like more than a simple patch 😄. Let me a day or two and I'll come back with something more polished.

[billy34]

Here it is

• added code to validate that protected and public paths start with "/" in src/app_config.rs
• added code to generate site_prefixed protected and public paths in src/webserver/oidc.rs. I added this code here as other oidc related url are also created in this file.
• updated configuration.md to make it clear that paths must start with "/" and will be prepended by site_prefix if needed

This may break existing configuration for those having set absolute paths that already include site_prefix.

[billy34]

• fixed code (site_prefix instead of site_prefix_trimmed)
• fixed formatting
• fixed clippy hints (pedantic mode). There remain one hint but not in the code I wrote.

[billy34]

Sorry thought I could fix these lines without installing the whole thing. My bad.
Dev environment set for the next fix/feature 😄

[lovasoa]

thanks, your work will be on lovasoa/sqlpage:main in a few minutes, and will be in the next release !

if there is anything else you want to change in sqlpage, open PRs and I'll be happy to provide guidance