Skip to content

Conversation

emlun
Copy link
Member

@emlun emlun commented Jan 23, 2023

Purpose

PR #8417 became a bit unwieldy, so reviewers requested these changes be extracted as a standalone first step.

This PR does not preserve the commit history from PR #8417, because that got very large and confusing. But if reviewers prefer, I can rebase these changes back onto the history from #8417.

Testing

See #8417 for a record of manual tests. I will add automated tests before exiting draft state.

Screenshots

Login page
Login page

About page while not logged in
About page while not logged in

Documentation

syncthing/docs#783

@emlun
Copy link
Member Author

emlun commented Jan 23, 2023

@tomasz1986 @calmh @bt90 @acolomb @imsodin Continuing from #8417 here.

@tomasz1986 Most of your comments #8417 (comment) should now be addressed. The documentation link still has no version number, but I think that's probably fine?

To your question on Android though, the conclusions in #8417 (comment) so far remain unchanged. I'm not sure where to start addressing that. Does Syncthing on Android run this same server binary? Are there any utilities for managing differences between the desktop and Android versions?

@imsodin
Copy link
Member

imsodin commented Jan 23, 2023

To your question on Android though, the conclusions in #8417 (comment) so far remain unchanged. I'm not sure where to start addressing that. Does Syncthing on Android run this same server binary? Are there any utilities for managing differences between the desktop and Android versions?

As far as I understand everything will work as expected in the app. To access the UI from outside the app, a config value needs setting. Correct? The config is controlled by the app, so that's a simple change there to add that new field and set it appropriately.

@emlun
Copy link
Member Author

emlun commented Jan 23, 2023

Yes - to be more precise, any client that sends the Authorization request header should continue working as it currently does. To access the UI from a standard browser (some of which don't send Authorization unless the site asks for it, even when explicitly included in the HTTP authority) you would need to set the new config flag. (That is on Android where you don't know the "password", none of this applies on desktop where you can just use the HTML login form.)

@imsodin
Copy link
Member

imsodin commented Jan 24, 2023

Ah right, it's not even about android itself, just old android where the login page doesn't work:

This seems to have poor browser support though. I've tested the code locally, and the GUI doesn't load in IE11 at all. Also, according to https://webauthn.me/browser-support and https://caniuse.com/webauthn, the minimum Android requirement is version 7+, while Syncthing itself currently supports Android 4.1.

So really a if androidVersion < 7 -> set basic auth flag in the app would be good - that's simple enough.

Copy link
Member

@imsodin imsodin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice to review with the small, refactor only commits separate first - thanks!
Haven't tested anything yet. Over all looks good, just a bunch of minor comments, and some of them entirely about style or potentially even bikeshedding.

CI is failing because the HTTPLogin test is failing as it checks for a 401 response. Could set the advanced option to test that, or instead check that it's not possible to access other things but index.html without auth.

@emlun
Copy link
Member Author

emlun commented Jan 30, 2023

So really a if androidVersion < 7 -> set basic auth flag in the app would be good - that's simple enough.

I don't think that's accurate, the flag would have to be set on all Android versions in order to access the GUI in a standard browser (the in-app GUI should continue to work without it, though). It's not enough that WebAuthn is supported on the platform, the user must also have actively enrolled a WebAuthn credential before they can use that to sign in.

@imsodin
Copy link
Member

imsodin commented Jan 30, 2023

I don't think that's accurate, the flag would have to be set on all Android versions in order to access the GUI in a standard browser (the in-app GUI should continue to work without it, though). It's not enough that WebAuthn is supported on the platform, the user must also have actively enrolled a WebAuthn credential before they can use that to sign in.

Right the original comment actually only talks about webauthn. So I guess I don't understand why plain password auth with the form added in this PR does not work on android. Webauthn doesn't, but that's jsut another option, isn't it?

@emlun
Copy link
Member Author

emlun commented Jan 30, 2023

So I guess I don't understand why plain password auth with the form added in this PR does not work on android.

It'll "work" technically - the form will be there and there will be a combination of inputs that'll get you through it. But as I understand, on Android the password is an API key, which a human cannot be expected to know. So the form will "work", but won't be useful.

Webauthn doesn't, but that's jsut another option, isn't it?

WebAuthn should work on recent enough versions of Android, but yes, it's an option and requires that the user has actively set it up first.

@imsodin
Copy link
Member

imsodin commented Jan 31, 2023

I might be confusing something badly here - this is my understanding: Api key and username/password are complimentary. If an API key is present in a request, that's used for auth, otherwise a session cookie (previous PW login) or finally actually use username/pw (sets the session cookie). Api key is typically used when accessing the rest API, and the android app uses that. This is unaffected by these changes. Then it also sets up a username/password, that a user can use when accessing the web UI outside of the app. From what I understand with the login form added here, username/pw login should still work regardless of the 401 challenge or not.

@emlun
Copy link
Member Author

emlun commented Jan 31, 2023

Ok, I could be mistaken, I'm really only parroting what @tomasz1986 said in #8417 (comment):

[...] the app currently uses a hard-coded username of "syncthing" and the password set to the API key. The user isn't allowed to change them (or they can try, but the two get reversed upon restart).

If that is inaccurate and the Android version still allows a normal password alongside the API key, then there should be no issue and the new advanced setting should only rarely be needed for niche use cases like having the credentials embedded in a URL.

Copy link
Member

@acolomb acolomb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glanced over the code and found a few nit-picks. But looks good in general, looking forward to testing this when the review comments are addressed.

@srwareham
Copy link

Does this PR still have a path to getting accepted? Would love to see it get picked up!

@emlun
Copy link
Member Author

emlun commented Apr 10, 2023

@srwareham Sorry, that's me being slow again! I'll try to get this finished up.

@bt90
Copy link
Contributor

bt90 commented Apr 10, 2023

@emlun no need to apologize. You donate your time and you are free to set your own pace 😉

A wizard is never late, nor is he early. He arrives precisely when he means to.

@bt90
Copy link
Contributor

bt90 commented Oct 6, 2023

As explained above, I think we can avoid both if the app sets the Authorization header explicitly in the loadurl("") call.

@calmh
Copy link
Member

calmh commented Oct 6, 2023

Fair enough; if we don't know of a use case for the setting we could/should skip it. We will likely get a "basic auth doesn't work any more" issue on release day, but ideally whoever is using that should use the API key or something instead...

@emlun
Copy link
Member Author

emlun commented Oct 6, 2023

In that case I believe this is now finally ready to merge. 😄 🎉

Copy link
Member

@calmh calmh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, that seems to be the case. We have a lot of time to find & iron out rough edges before next release, including removing settings or adding endpoints. Thanks @emlun for the hard work and exemplary patience along the way.

@calmh calmh merged commit 8294870 into syncthing:main Oct 6, 2023
@emlun emlun deleted the login-form branch October 6, 2023 11:18
@emlun
Copy link
Member Author

emlun commented Oct 6, 2023

Likewise thank you all for your equally exemplary patience with me. 😄

@tomasz1986
Copy link
Member

tomasz1986 commented Oct 6, 2023

The app would need to modify the config after the update. And we could get rid of the setting entirely as it's only a workaround for embedded browsers.

Just for clarification, the basic auth option was added by @emlun in the original PR after I asked for an ability to still log in using the http://username:password@localhost:8384 scheme. It had nothing to do with the Android app or embedded browsers.

I have all my Syncthing devices on the local network bookmarked like this, so that I don't need to input any credentials myself while still having the GUI protected from outside access.

@bt90
Copy link
Contributor

bt90 commented Oct 6, 2023

Isn't that something better solved with passwordmanager? But yeah, there might be other usecases out there which still need this.

@tomasz1986
Copy link
Member

tomasz1986 commented Oct 6, 2023

Isn't that something better solved with passwordmanager?

The direct link is still faster, but on the desktop, I could probably get by without it, however I also need to access the GUI via mobile browsers on Android, and I haven't found any seamless way to integrate a password manager with those (except for saving passwords in the browser itself which I'm not a fan of).

@bt90
Copy link
Contributor

bt90 commented Oct 6, 2023

Apparently the fix might be a lot easier. WebView allows to set custom headers in loadurl("")

https://stackoverflow.com/a/41842452

Works. syncthing/syncthing-android#1982

calmh pushed a commit that referenced this pull request Oct 10, 2023
…ions (#9159)

This is motivated by the Android app:
syncthing/syncthing-android#1982 (comment)

The planned fix in response to basic auth behaviour changing in #8757
was to add the `Authorization` header when opening the WebView, but it
turns out the function used only applies the header to the initial page
load, not any subsequent script loads or AJAX calls. The
`basicAuthAndSessionMiddleware` checks for no-auth exceptions before
checking the `Authorization` header, so the header has no effect on the
initial page load since the `/` path is a no-auth exception. Thus the
Android app fails to log in when opening the WebView.

This changes the order of checks in `basicAuthAndSessionMiddleware` so
that the `Authorization` header is always checked if present, and a
session cookie is set if it is valid. Only after that does the
middleware fall back to checking for no-auth exceptions.

`api_test.go` has been expanded with additional checks:
- Check that a session cookie is set whenever correct basic auth is
provided.
- Check that a session cookie is not set when basic auth is incorrect.
- Check that a session cookie is not set when authenticating with an API
token (either via `X-Api-Key` or `Authorization: Bearer`).

And an additional test case:
- Check that requests to `/` always succeed, but receive a session
cookie when correct basic auth is provided.

I have manually verified that
- The new assertions fail if the `createSession` call is removed in
`basicAuthAndSessionMiddleware`.
- The new test cases in e6e4df4 fail
before the change in 0e47d37 is
applied.
calmh pushed a commit that referenced this pull request Oct 10, 2023
…ptions (#9159)

This is motivated by the Android app:
syncthing/syncthing-android#1982 (comment)

The planned fix in response to basic auth behaviour changing in #8757
was to add the `Authorization` header when opening the WebView, but it
turns out the function used only applies the header to the initial page
load, not any subsequent script loads or AJAX calls. The
`basicAuthAndSessionMiddleware` checks for no-auth exceptions before
checking the `Authorization` header, so the header has no effect on the
initial page load since the `/` path is a no-auth exception. Thus the
Android app fails to log in when opening the WebView.

This changes the order of checks in `basicAuthAndSessionMiddleware` so
that the `Authorization` header is always checked if present, and a
session cookie is set if it is valid. Only after that does the
middleware fall back to checking for no-auth exceptions.

`api_test.go` has been expanded with additional checks:
- Check that a session cookie is set whenever correct basic auth is
provided.
- Check that a session cookie is not set when basic auth is incorrect.
- Check that a session cookie is not set when authenticating with an API
token (either via `X-Api-Key` or `Authorization: Bearer`).

And an additional test case:
- Check that requests to `/` always succeed, but receive a session
cookie when correct basic auth is provided.

I have manually verified that
- The new assertions fail if the `createSession` call is removed in
`basicAuthAndSessionMiddleware`.
- The new test cases in e6e4df4 fail
before the change in 0e47d37 is
applied.
@calmh calmh added this to the v1.26.0 milestone Oct 12, 2023
imsodin pushed a commit to syncthing/syncthing-android that referenced this pull request Oct 14, 2023
As preparation for syncthing/syncthing#8757. See
syncthing/syncthing#8757 (comment)

The app sends the `Authorization: Basic <base64-ui-credentials>` HTTP
header in the initial request when loading the Web UI to skip the login
form.
calmh added a commit that referenced this pull request Oct 14, 2023
* main:
  lib/fs: Properly handle Windows deduplicated files (fixes #9120) (#9168)
  cmd/ursrv: Replace "2006-01-02" with time.DateOnly (#9157)
  lib/api: Check basic auth (and set session cookie) before noauth exceptions (#9159)
  gui, man, authors: Update docs, translations, and contributors
  lib/api: Better handle %s templates in LDAP strings (fixes #9072) (#9155)
  docker: Allow start even if chown fails (fixes #9133) (#9152)
  lib/model: Verify versioning on configuration reload (fixes #9106) (#9154)
  cmd/stdiscosrv: Handle unescaped cert header from Traefik (fixes #9143) (#9153)
  Add oxford comma (#9137)
  Add HTML login form (fixes #4137) (#8757)
  gui: Fix favicon status (fixes #9149) (#9150)
  cmd/stdiscosrv: Slightly tweak replication settings
  build: Update deps
  build: Run release steps for workflow_dispatch as well
  cmd/ursrv: Add linuxserver.io detection (#9145)
  lib/config: Improve parsing of gui-address overrides (#9144)
  gui, man, authors: Update docs, translations, and contributors
claraphyll added a commit to claraphyll/syncthing that referenced this pull request Oct 15, 2023
commit a405c21
Author: Jakob Borg <jakob@kastelo.net>
Date:   Sat Oct 14 12:29:53 2023 +0200

    cmd/stdiscosrv: Only attempt unescaping when there are %-encodings in the header (fixes syncthing#9143)

commit dc6a10d
Author: Jakob Borg <jakob@kastelo.net>
Date:   Wed Sep 20 08:52:33 2023 +0200

    cmd/stcrashreceiver: Aggregate slice out of bounds errors

commit d4c2acf
Author: Jakob Borg <jakob@kastelo.net>
Date:   Wed Sep 20 08:39:01 2023 +0200

    cmd/stcrashreceiver: Propagate synthetic user ID for crashes

commit 483ecad
Author: Jakob Borg <jakob@kastelo.net>
Date:   Sat Oct 14 12:18:27 2023 +0200

    build: Update dependencies

commit 9553365
Author: Eric P <eric@kastelo.net>
Date:   Wed Oct 11 14:40:55 2023 +0200

    lib/fs: Properly handle Windows deduplicated files (fixes syncthing#9120) (syncthing#9168)

    ### Purpose

    Deduplicated files are apparently considered 'irregular' under the hood,
    this causes them to simply be ignored by Syncthing. This change is more
    of a workaround than a proper fix, as the fix should probably happen in
    the underlying libraries? - which may take some time. In the meanwhile,
    this change should make deduplicated files be treated as regular files
    and be indexed and synced as they should.

    ### Testing

    Create some volume where deduplication is turned on (see the relevant
    issue for details, including a proper description of how to reproduce
    it). Prior to this change, the deduplicated files were simply ignored
    (even by the indexer). After this change, the deduplicated files are
    being index and synced properly.

commit 5eb2058
Author: orangekame3 <miya.org.0309@gmail.com>
Date:   Wed Oct 11 19:32:19 2023 +0900

    cmd/ursrv: Replace "2006-01-02" with time.DateOnly (syncthing#9157)

    This commit replaces "2006-01-02" to time.DateOnly. time.DateOnly is
    introduced since Go1.20

commit ea1ea36
Author: Emil Lundberg <emil@emlun.se>
Date:   Tue Oct 10 07:48:35 2023 +0200

    lib/api: Check basic auth (and set session cookie) before noauth exceptions (syncthing#9159)

    This is motivated by the Android app:
    syncthing/syncthing-android#1982 (comment)

    The planned fix in response to basic auth behaviour changing in syncthing#8757
    was to add the `Authorization` header when opening the WebView, but it
    turns out the function used only applies the header to the initial page
    load, not any subsequent script loads or AJAX calls. The
    `basicAuthAndSessionMiddleware` checks for no-auth exceptions before
    checking the `Authorization` header, so the header has no effect on the
    initial page load since the `/` path is a no-auth exception. Thus the
    Android app fails to log in when opening the WebView.

    This changes the order of checks in `basicAuthAndSessionMiddleware` so
    that the `Authorization` header is always checked if present, and a
    session cookie is set if it is valid. Only after that does the
    middleware fall back to checking for no-auth exceptions.

    `api_test.go` has been expanded with additional checks:
    - Check that a session cookie is set whenever correct basic auth is
    provided.
    - Check that a session cookie is not set when basic auth is incorrect.
    - Check that a session cookie is not set when authenticating with an API
    token (either via `X-Api-Key` or `Authorization: Bearer`).

    And an additional test case:
    - Check that requests to `/` always succeed, but receive a session
    cookie when correct basic auth is provided.

    I have manually verified that
    - The new assertions fail if the `createSession` call is removed in
    `basicAuthAndSessionMiddleware`.
    - The new test cases in e6e4df4 fail
    before the change in 0e47d37 is
    applied.

commit 6e4574a
Author: Syncthing Release Automation <release@syncthing.net>
Date:   Mon Oct 9 03:45:35 2023 +0000

    gui, man, authors: Update docs, translations, and contributors

commit 3d0da5a
Author: Jakob Borg <jakob@kastelo.net>
Date:   Sat Oct 7 04:29:53 2023 +0200

    lib/api: Better handle %s templates in LDAP strings (fixes syncthing#9072) (syncthing#9155)

    Also add some escaping for good measure.

commit 9f8e696
Author: Jakob Borg <jakob@kastelo.net>
Date:   Sat Oct 7 04:12:07 2023 +0200

    docker: Allow start even if chown fails (fixes syncthing#9133) (syncthing#9152)

commit a64ae36
Author: Jakob Borg <jakob@kastelo.net>
Date:   Sat Oct 7 04:09:51 2023 +0200

    lib/model: Verify versioning on configuration reload (fixes syncthing#9106) (syncthing#9154)

commit 690b553
Author: Jakob Borg <jakob@kastelo.net>
Date:   Sat Oct 7 04:09:07 2023 +0200

    cmd/stdiscosrv: Handle unescaped cert header from Traefik (fixes syncthing#9143) (syncthing#9153)

commit 2f6187d
Author: DeflateAwning <11021263+DeflateAwning@users.noreply.github.com>
Date:   Fri Oct 6 09:25:28 2023 -0600

    Add oxford comma (syncthing#9137)

    Co-authored-by: André Colomb <src@andre.colomb.de>

commit 8294870
Author: Emil Lundberg <emil@emlun.se>
Date:   Fri Oct 6 13:00:58 2023 +0200

    Add HTML login form (fixes syncthing#4137) (syncthing#8757)

commit ac2e444
Author: bt90 <btom1990@googlemail.com>
Date:   Fri Oct 6 12:27:13 2023 +0200

    gui: Fix favicon status (fixes syncthing#9149) (syncthing#9150)

commit 4f6b86a
Author: Jakob Borg <jakob@kastelo.net>
Date:   Wed Oct 4 11:36:49 2023 +0200

    cmd/stdiscosrv: Slightly tweak replication settings

commit 516c057
Author: Jakob Borg <jakob@kastelo.net>
Date:   Tue Oct 3 10:00:16 2023 +0200

    build: Update deps

commit d644dce
Author: Jakob Borg <jakob@kastelo.net>
Date:   Tue Oct 3 09:33:52 2023 +0200

    build: Run release steps for workflow_dispatch as well

commit 7c57988
Author: bt90 <btom1990@googlemail.com>
Date:   Mon Oct 2 12:48:04 2023 +0200

    cmd/ursrv: Add linuxserver.io detection (syncthing#9145)

    Detect linuxserver

commit 296db31
Author: Jakob Borg <jakob@kastelo.net>
Date:   Mon Oct 2 08:40:03 2023 +0200

    lib/config: Improve parsing of gui-address overrides (syncthing#9144)

    improve parsing of gui-address overrides

    make checks for whether the gui-address is overridden consistent by
    checking whether the environment variable is set and not an empty
    string. the `Network()` function however checked for the inclusion of
    a slash instead of the presence of any characters. If the config file's
    gui address was set to a unix socket and the gui override to a tcp
    address, then the function would have wrongly returned "unix".

    the `url("")` function always returned the config file's gui address if a
    unix socket was configured, even if an override was specified.

    the `url("")` function wrongly formatted unix addresses. the http(s)
    protocol was used as the sheme and the path was percent escaped. because
    of the previous bug, this could only be triggered if the config file's
    gui address was tcp and an unix socket override was given.

    simplify the `useTLS()` function's codepath for overrides.

    Co-authored-by: digital <didev@dinid.net>

commit a8486b0
Author: Syncthing Release Automation <release@syncthing.net>
Date:   Mon Oct 2 03:45:41 2023 +0000

    gui, man, authors: Update docs, translations, and contributors

commit f8a7a03
Author: bt90 <btom1990@googlemail.com>
Date:   Fri Sep 29 17:42:44 2023 +0200

    cmd/ursrv: Fix f-droid detection (syncthing#9142)

    Fix f-droid detection

commit ceae56a
Author: bt90 <btom1990@googlemail.com>
Date:   Fri Sep 29 16:34:28 2023 +0200

    cmd/ursrv: Support new android build user (syncthing#9141)

    Support new android build user

commit dcafd6e
Author: DeflateAwning <11021263+DeflateAwning@users.noreply.github.com>
Date:   Thu Sep 28 03:55:48 2023 -0600

    readme: Style fixes, add security note (syncthing#9136)

commit 8619a03
Author: Jakob Borg <jakob@kastelo.net>
Date:   Mon Sep 25 21:50:17 2023 +0200

    build: Update Actions

commit b91d771
Author: Jakob Borg <jakob@kastelo.net>
Date:   Mon Sep 25 21:45:57 2023 +0200

    Update dependencies (syncthing#9129)

    And some QUIC API changes, of course.

commit 9940c91
Author: d-volution <49024624+d-volution@users.noreply.github.com>
Date:   Mon Sep 25 21:42:27 2023 +0200

    gui: Scroll to bottom by clicking message in log viewer (syncthing#9128)

commit 80a577b
Author: tomasz1986 <twilczynski@naver.com>
Date:   Mon Sep 25 21:34:19 2023 +0200

    gui: Show if device is untrusted in the main GUI (syncthing#9116)

    Add a new entry to the unfolded device info to inform the user that the
    device has been marked as "untrusted" and all folders shared with it
    have to be password-protected or already Receive Encrypted.

    Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>

commit d672175
Author: tomasz1986 <twilczynski@naver.com>
Date:   Mon Sep 25 21:33:16 2023 +0200

    gui: Show if device has Auto Accept enabled in the main GUI (syncthing#9118)

    Add a new entry to the unfolded device info to inform the user that the
    device has Auto Accept enabled.

    Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>

commit a44b31d
Author: tomasz1986 <twilczynski@naver.com>
Date:   Mon Sep 25 21:17:57 2023 +0200

    gui: Fix body padding infinitely increasing due to overlapping modals (ref syncthing#9063) (syncthing#9078)

    Opening and hiding multiple modals at the same time as well as opening a
    modal before fully hiding the previous one can lead to the body padding
    infinitely increasing by the scrollbar width each time, with the only
    way to fix it being refreshing the GUI.

    Therefore, always try to ensure to open and hide multiple modals one by
    one, and also that the previous modal has fully been hidden before
    proceeding to open the next one. The most common case when this problem
    happens is when saving config changes which displays a GUI blocking
    modal that overlaps, e.g. with folder or device modals that have not
    been hidden yet.

    Ref: twbs/bootstrap#3902 (comment)

    Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>

commit 70065e6
Author: Martin Polehla <p0l0us@users.noreply.github.com>
Date:   Mon Sep 25 16:17:01 2023 +0200

    gitignore: All exe files, no editor configs (syncthing#9126)

commit adbb3ed
Author: Syncthing Release Automation <release@syncthing.net>
Date:   Mon Sep 25 03:45:38 2023 +0000

    gui, man, authors: Update docs, translations, and contributors

commit 6ed9c0c
Author: Jakob Borg <jakob@kastelo.net>
Date:   Sun Sep 24 19:23:49 2023 +0200

    lib/config: Accept pre-hashed password (fixes syncthing#9123) (syncthing#9124)

commit 19bbf4f
Author: tomasz1986 <twilczynski@naver.com>
Date:   Fri Sep 22 07:39:16 2023 +0200

    gui: Add missing $scope in editDeviceUntrustedChanged function (syncthing#9117)

    Because $scope is missing, there are JavaScript errors when ticking and
    unticking the "Untrusted" checkbox in the Advanced tab of the Edit
    Device modal.

    Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>

commit cf46bf0
Author: bt90 <btom1990@googlemail.com>
Date:   Wed Sep 20 11:23:48 2023 +0200

    lib/connections: Fix transport type detection for QUIC (fixes syncthing#8274) (syncthing#9114)

    Check remote address

commit 051cbdc
Author: Jakob Borg <jakob@kastelo.net>
Date:   Wed Sep 20 09:04:47 2023 +0200

    lib/fs, lib/model: Be careful about potentially negative durations (fixes syncthing#9112) (syncthing#9113)

    I don't really understand under what circumstances, but sometimes these
    calls panic with a "panic: counter cannot decrease in value" because the
    value passed to Add() was negative.

commit 58d1f3a
Author: Syncthing Release Automation <release@syncthing.net>
Date:   Mon Sep 18 03:45:31 2023 +0000

    gui, man, authors: Update docs, translations, and contributors

commit c9dfd75
Author: tomasz1986 <twilczynski@naver.com>
Date:   Tue Sep 12 15:02:37 2023 +0200

    gui: Block GUI when saving changes only if necessary (ref syncthing#9063) (syncthing#9079)

    Currently, the UI is always blocked from modifications when changes are
    being saved, even if the save process takes very little time. This leads
    to a situation where showing and closing the blocking modal can take
    more time than is actually required to perform the whole operation. The
    modal opening and closing very quickly can also cause the screen to
    flash for a brief moment, leading to visual discomfort.

    Because of this, wait for at least 200 ms and only show the blocking
    modal if the changes have not been saved until then yet. The value of
    200 ms is loosely based on [1] which states that 'a delay of 0.2–1.0
    seconds does mean that users notice the delay and thus feel the computer
    is "working" on the command, as opposed to having the command be a
    direct effect of the users' actions.' Additionally, the delay must not
    be too long, because the main purpose of the blocking modal is to
    prevent the user from making further changes, and a longer delay would
    possibly allow to do so in that brief amount of time as long as the user
    is quick enough with their input.

    [1] https://nngroup.com/articles/response-times-3-important-limits

    Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>

commit f47de83
Author: Jakob Borg <jakob@kastelo.net>
Date:   Tue Sep 12 14:48:15 2023 +0200

    lib/protocol: Ensure starting & closing a connection are exclusive (fixes syncthing#9102) (syncthing#9103)

    In principle a connection can close while it's in progress with
    starting, and then it's undefined if we wait for goroutines to exit etc.
    With this change, we will wait for start to complete before starting to
    stop everything.

commit caedb19
Author: tomasz1986 <twilczynski@naver.com>
Date:   Tue Sep 12 14:47:31 2023 +0200

    gui: Remove unused hard-coded styles from Recent Changes modal (syncthing#9101)

    gui: Remove unused hard-coded styles from globalChangesModalView modal

    Currently, the globalChangesModalView modal has hardcoded th and td
    styles. However, they are not even used in the modal itself, because
    Bootstrap overrides them with its own styles for these elements in the
    same modal. Yet, when hard-coded like that, these styles can conflict
    with other table elements in the GUI. Thus, remove them completely.

    Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>

commit e860d3b
Author: bt90 <btom1990@googlemail.com>
Date:   Tue Sep 12 14:34:30 2023 +0200

    lib/connections: Make assumptions about isLAN when interface address listing fails (syncthing#9093)

commit ed66fba
Author: bt90 <btom1990@googlemail.com>
Date:   Tue Sep 12 14:28:17 2023 +0200

    lib/beacon, lib/discover: Send IPv4 limited broadcast when address listing fails (fixes syncthing#1628) (syncthing#9087)

commit 415f320
Author: Jakob Borg <jakob@kastelo.net>
Date:   Tue Sep 12 14:08:59 2023 +0200

    build: Update dependencies

commit 4812600
Author: Jakob Borg <jakob@kastelo.net>
Date:   Mon Sep 11 23:10:18 2023 +0200

    lib/versioner: Don't complain when folder is stopping (syncthing#9097)

commit 5ff11ce
Author: Jakob Borg <jakob@kastelo.net>
Date:   Mon Sep 11 14:59:48 2023 +0200

    gui: Add help link for numConnections (syncthing#9082)

commit 5415727
Author: tomasz1986 <twilczynski@naver.com>
Date:   Mon Sep 11 05:50:23 2023 +0200

    gui: Add missing translation related to Number of Connections (ref syncthing#8918) (syncthing#9095)

    Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>

commit e38679d
Author: Syncthing Release Automation <release@syncthing.net>
Date:   Mon Sep 11 03:45:45 2023 +0000

    gui, man, authors: Update docs, translations, and contributors

commit f25a169
Author: Jakob Borg <jakob@kastelo.net>
Date:   Wed Sep 6 21:10:58 2023 +0200

    build: Go 1.21.1 or higher

commit 06ac10e
Author: bt90 <btom1990@googlemail.com>
Date:   Wed Sep 6 14:36:00 2023 +0200

    cmd/stdiscosrv: Deduplicate addresses (fixes syncthing#8482) (syncthing#9080)

commit 7c0223b
Author: Jakob Borg <jakob@kastelo.net>
Date:   Wed Sep 6 13:11:20 2023 +0200

    lib/build: Next version is the Gold Grasshopper

commit c6334e6
Author: Jakob Borg <jakob@kastelo.net>
Date:   Wed Sep 6 12:52:01 2023 +0200

    all: Support multiple device connections (fixes syncthing#141) (syncthing#8918)

    This adds the ability to have multiple concurrent connections to a single device. This is primarily useful when the network has multiple physical links for aggregated bandwidth. A single connection will never see a higher rate than a single link can give, but multiple connections are load-balanced over multiple links.

    It is also incidentally useful for older multi-core CPUs, where bandwidth could be limited by the TLS performance of a single CPU core -- using multiple connections achieves concurrency in the required crypto calculations...

    Co-authored-by: Simon Frei <freisim93@gmail.com>
    Co-authored-by: tomasz1986 <twilczynski@naver.com>
    Co-authored-by: bt90 <btom1990@googlemail.com>

commit 38bbdeb
Author: Jakob Borg <jakob@kastelo.net>
Date:   Tue Sep 5 09:52:19 2023 +0200

    build: Use actions/checkout@v4

commit e80d048
Author: Jakob Borg <jakob@kastelo.net>
Date:   Tue Sep 5 09:47:51 2023 +0200

    build: Minor dependency update

commit 4138e22
Author: Syncthing Release Automation <release@syncthing.net>
Date:   Mon Sep 4 03:45:39 2023 +0000

    gui, man, authors: Update docs, translations, and contributors

commit c42c0e7
Author: Maximilian <maxi.rostock@outlook.de>
Date:   Sun Sep 3 17:03:27 2023 +0200

    lib/connections: Fix WANAddresses returning only unspecified IPs (ref syncthing#9010) (syncthing#9073)

    Avoids taking the address of the same variable twice.

commit 5118538
Author: Jakob Borg <jakob@kastelo.net>
Date:   Sat Sep 2 16:42:46 2023 +0200

    lib/model: Refactor folderRunners to use a serviceMap (syncthing#9071)

    Instead of separately tracking the token.

    Also changes serviceMap to have a channel version of RemoveAndWait, so
    that it's possible to do the removal under a lock but wait outside of
    the lock. And changed where we do that in connection close, reversing
    the change that happened when I added the serviceMap in 40b3b9a.

commit 4d93648
Author: tomasz1986 <twilczynski@naver.com>
Date:   Sat Sep 2 12:19:18 2023 +0200

    gui: Don't hide default values for folders and devices (syncthing#8987)

    Currently, some of the information for folders and devices displayed in
    the GUI relies on arbitrary values that come pre-set as defaults on a
    fresh Syncthing installation, i.e. if the value matches the default, it
    is hidden, and if does not, then it is displayed.

    With this change, the GUI always displays all information regardless
    of their value, making the overall experience more consistent and
    predictable.

    Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>

commit 29f100c
Author: tomasz1986 <twilczynski@naver.com>
Date:   Fri Sep 1 11:15:20 2023 +0200

    gui: Fix File Versioning icon to match in all places (syncthing#9070)

    Currently, different icons are used for File Versioning when displayed
    in the unfolded folder info in the main part of the GUI, and the icon
    used in the Edit Folder modal. This changes the main GUI icon to match
    the icon used in the modal.

    Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>

commit cd98a43
Author: tomasz1986 <twilczynski@naver.com>
Date:   Fri Sep 1 11:14:28 2023 +0200

    gui: Fix Logs modal icon to match header icon (ref syncthing#9067) (syncthing#9069)

    The Logs icon was changed in [1] in the header, however the icon used in
    the modal was left out. This changes it, so that the header and the
    modal icons match.

    [1] 2abba1d

    Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>

commit 4bf9823
Author: Jakob Borg <jakob@kastelo.net>
Date:   Fri Sep 1 08:35:30 2023 +0200

    build: Be more subtle about cross compilation errors

    Summarize platforms that fail to build, without overloading the build
    log with errors that we anyway ignore. (Currently freebsd/riscv64 fails
    to build.)

commit 29056d5
Author: Jakob Borg <jakob@kastelo.net>
Date:   Fri Sep 1 08:39:15 2023 +0200

    build: Update dependencies (syncthing#9068)

commit 2abba1d
Author: tomasz1986 <twilczynski@naver.com>
Date:   Fri Sep 1 08:18:30 2023 +0200

    gui: Remove footer and move links to header (fixes syncthing#5607) (syncthing#9067)

    * gui: Remove footer and move links to header (fixes syncthing#5607)

    Currently, the footer is always present and takes space at the bottom of
    the GUI. However, the links listed there are not part of everyday user
    interaction, and as such, they unnecessarily clutter the page, reducing
    the usable screen space. Thus, transform the current Help link in the
    header into a Help dropdown menu, and move the links from the footer
    into it.

    Also apply the following tweaks:

    1. Move the About dialog from Actions to Help.
    2. Add an Introduction (to the GUI) link to Help.
    3. Change the Support icon from a question mark to a group of people.
    4. Change the Changelog and About icons to a filled version to match the
       other icons better.
    5. Use a source code icon for Source Code instead of a wrench icon, and
       move the wrench icon to Logs. This is done to prevent Changelog and
       Logs from using the same icon.
    6. Update all dropdown icons' Fork Awesome styles to "fa fa-fw <icon>".

    Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>

    * a few more Fork Awesome style updates

    ---------

    Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>

commit 325b3b1
Author: tomasz1986 <twilczynski@naver.com>
Date:   Fri Sep 1 07:22:04 2023 +0200

    gui: Fix lastSeenDays error due to undefined deviceStats when adding new devices (ref syncthing#8730) (syncthing#9066)

commit 03590e5
Author: tomasz1986 <twilczynski@naver.com>
Date:   Thu Aug 31 22:16:59 2023 +0200

    gui: Automatically select device ID on click (ref syncthing#8544) (syncthing#9065)

    The CSS method to select device IDs on click was added in [1]. However,
    it was later mistakenly overwritten by [2]. This commit fixes the
    regression and also applies the same behaviour to the Edit Device modal
    which was omitted in the original commit.

    [1] 5baf5fe
    [2] 5e384c9

    Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>

commit 95b3c26
Author: tomasz1986 <twilczynski@naver.com>
Date:   Thu Aug 31 17:11:03 2023 +0200

    gui: Prevent modifications when saving changes (fixes syncthing#9019) (syncthing#9063)

commit 3e5f0b1
Author: tomasz1986 <twilczynski@naver.com>
Date:   Thu Aug 31 07:22:24 2023 +0200

    gui: Show in GUI if limitBandwidthInLan is enabled (syncthing#9062)

commit 3130af3
Author: Jakob Borg <jakob@kastelo.net>
Date:   Wed Aug 30 21:58:34 2023 +0200

    lib/upgrade: Enable HTTP/2 for upgrade checks (syncthing#9060)

commit abd89f1
Author: Jakob Borg <jakob@kastelo.net>
Date:   Wed Aug 30 21:58:05 2023 +0200

    lib/discover: Enable HTTP/2 for global discovery requests (syncthing#9059)

    By creating the http.Transport and tls.Configuration ourselves we
    override some default behavior and end up with a client that speaks only
    HTTP/1.1.

    This adds a call to http.ConfigureTransport to do the relevant magic to
    enable HTTP/2.

    Also tweaks the keepalive settings to be a little kinder to the
    server(s).

commit a80e6be
Author: Jakob Borg <jakob@kastelo.net>
Date:   Wed Aug 30 09:36:27 2023 +0200

    cmd/stdiscosrv: Streamline context handling

commit acc532f
Author: Jakob Borg <jakob@kastelo.net>
Date:   Wed Aug 30 09:09:50 2023 +0200

    cmd/stdiscosrv: Explicitly enable HTTP/2

    The server supports it, but it's not negotiated unless explicitly
    allowed in the TLS config NextProtos.

commit 3cc3fb7
Author: Syncthing Release Automation <release@syncthing.net>
Date:   Mon Aug 28 03:45:57 2023 +0000

    gui, man, authors: Update docs, translations, and contributors
acolomb pushed a commit that referenced this pull request Oct 15, 2023
This was an oversight in #8757: the new "Log out" button is always shown
in the "Actions" menu, even when authentication is not enabled.
acolomb pushed a commit to syncthing/docs that referenced this pull request Nov 5, 2023
@emlun emlun mentioned this pull request Aug 26, 2024
10 tasks
calmh pushed a commit that referenced this pull request Sep 22, 2024
### Purpose

Since #8757, the Syncthing GUI now has an unauthenticated state. One
consequence of this is that `$scope.versionBase()` is not initialized
while unauthenticated, which causes the `docsURL` function to truncate
links to just `https://docs.syncthing.net`/, discarding the section
path. This currently affects at least the "Help > Introduction" link
reachable both while logged in and not. The issue is exacerbated in
#9175 where we sometimes want
to show additional contextual help links from the login page to
particular sections of the docs.

I don't think it's any worse to try to preserve the section path even
without an explicit version tag, than to fall back to just the host and
lose all context the link was attempting to provide.

### Testing

- On commit b1ed280 (before):
  - Open the GUI, set a username and log out.
- Open the "Help" drop-down. The "Introduction" item links to:
https://docs.syncthing.net/
  - Log in.
- Open the "Help" drop-down. The "Introduction" item links to:
https://docs.syncthing.net/v1.27.10/intro/gui
- On commit 44fef31 (after):
  - Open the GUI, set a username and log out.
- Open the "Help" drop-down. The "Introduction" item links to:
https://docs.syncthing.net/intro/gui
  - Log in.
- Open the "Help" drop-down. The "Introduction" item links to:
https://docs.syncthing.net/v1.27.10/intro/gui

### Screenshots

This is a GUI change, but affecting only URLs in the markup with no
visual changes.


### Drawbacks

If a `docsURL` call generates a versionless link to a docs page that
doesn't exist on https://docs.syncthing.net - presumably because
Syncthing is not the latest version and links to a deleted page? - then
this will lead to GitHub's generic 404 page with no link to the
Syncthing docs root. Before, any versionless link would also be a
pathless link, leading to the Syncthing docs root instead of a 404 page.
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Oct 5, 2024
@syncthing syncthing locked and limited conversation to collaborators Oct 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants