feat(web): request a root authentication method after selecting a product #1787

jreidinger · 2024-11-27T13:40:54Z

Problem

To ensure privileges in the installed systems, users must define at least one authentication method for the root user. This can easily be accomplished by visiting the "Users" page. However, since it is not fully mandatory right now, users might overlook it and end up with an unmanageable system.

Solution

As soon as the installer detects an issue within the users' scope, it will display a new page with a form to set at least one root authentication method. It should help to mitigate the risk of ending up with a system lacking access to the administrative user.

Although not perfect, as users could later bypass the installer by adding a regular user and then removing all root authentication methods to avoid detection of an issue related to the users, this is a good step towards a future where the first regular user could also have administrative privileges (a decision to be made).

After commit fe28059, the above workaround was discarded in favor of making a root authentication method mandatory in Agama web interface by forcing users to set least one of them as soon as it detects there is none. I.e., at least once after selecting a product for the first time.

Users can still change their chosen method or value later, but they always must provide at least one before installation. This approach is more robust and consistent than the previous implementation. It's also better aligned with the purpose of introducing this additional step: to ensure the installed system has an administrative user properly set.

Notes for reviewers

Reviewers, in addition to testing the feature manually, please keep the following aspects in mind.

[UX] Text on the screen: suggest as many improvements as you can, but keep in mind the need to provide all relevant context and instructions for users who will come across this page right after selecting a product.
[UX] File Upload: As you can see, the file upload component has been tweaked slightly to improve the keyboard experience. It will certainly require further internal improvements, but we believe it's a good starting point.
[DX] Naming: If you have better suggestions for file and route names, please share them. I'm (@dgdavid) not happy with the current ones either.
A cache optimistic update has been used to avoid a navigation back to the form because of routing before updating query cache with the server response. See commit fe28059
There are two commits for using "encryptedPassword" in every place instead of "encryptedPassword" in some of them and "passwordEncrypted" in others. See 00da7b3 and bf10992

Testing

Tested manually
Added unit tests

Screenshots

Not filled form	Partially filled form

Related to https://trello.com/c/YdZIRHoo (internal link)

Which will be initially shown after selecting a product if user issues are found.

Useful for routes "outside" of "installation settings context".

Still missing the logic for validating and sending data provided by user.

And also * add a unit testing for ensuring it behaves as expected. * fix some typos found in previously touched files.

To keep consistency and not having both.

web/src/queries/users.ts

For consistency with the rest of the code. Related to 00da7b3

web/src/App.tsx

web/src/components/core/Page.tsx

jreidinger · 2024-11-28T15:56:36Z

web/src/components/users/RootAuthMethodsPage.tsx

+  // TRANSLATORS: %s will be replaced by a link with the text "upload".
+  const [sshKeyStartHelperText, sshKeyEndHelperText] = _(
+    "Write, paste, drop, or %s a SSH public key file in the above textarea.",
+  ).split("%s");


will this work also for right to left locales?

Great question! Honestly, I don't know. But for now, I’m just using the interpolation we've documented at https://agama-project.github.io/docs/devel/i18n#formatting-texts.

That said, I really appreciate you raising concerns about RTL languages. While I did try to approach things with RTL in mind from the start, I’m afraid we're still far of having a fully RTL-compatible interface. There are so many parts involved, and even the CSS we're using is key.

Don't get me wrong. I’m not suggesting we stop thinking about RTL just because we don’t have everything in place yet. However, I'd keep things as they are for now and take notes on how to improve these translations to make them RTL-compatible, and tackle them all at once when we can.

Thanks so much for your input!

!olleH
RTL expert here :-)

This split("%s") technique is fine with respect to bidirectional languages: there are just letters, no numbers or punctuation. Refer to https://github.com/mvidner/bidi-test for examples of failures related to that.

If done incorrectly ("Write, paste, drop, or" + link + "a SSH public key file in the above textarea.") it would be a problem because of grammar, not text direction:
In Japanese, the translation will come out like this:

"In the above area, a SSH key write, paste, drop or %s".

Our implementation will handle that fine.

Thanks a lot for the explanation @mvidner!

By fixing typos and rewording documentation props. Also drops a no longer needed TODO at src/App.tsx

Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>

web/src/components/users/RootAuthMethodsPage.tsx

Please, fix the CI issues.

Because the mock was affecting to the entire test suite. They will be reintroduced once the react-router-dom mock is better done.

web/src/components/users/RootAuthMethodsPage.test.tsx

mvidner · 2024-12-02T08:44:09Z

web/src/components/users/RootAuthMethodsPage.tsx

+            description={
+              <p className={textStyles.fontSizeXl}>
+                {_(
+                  "You must define at least one authentication method for the root user. You can still edit them anytime before the installation.",


will the new root password screen be localizable?

@bittin yes of course, as you can see here.
If by any chance it does not work then we may have broken things in the recent refactoring, #1777.

@mvidner ah alright thanks, have not seen anything pop up at: https://l10n.opensuse.org/projects/agama/-/sv/ yet

bittin · 2024-12-03T06:30:11Z

@mvidner @imobachgs translated to Swedish now :) this morning

bittin · 2024-12-05T08:20:25Z

The screen is not fully translated

dgdavid · 2024-12-05T09:05:05Z

The screen is not fully translated

What is missing? Can you provide an screenshot or so?

bittin · 2024-12-05T09:07:26Z

@dgdavid

Still a lot of English here

mvidner · 2024-12-05T10:08:34Z

I will check the missing translations

mvidner · 2024-12-05T11:08:37Z

@bittin what build of Agama are you using?

bittin · 2024-12-05T11:21:26Z

@mvidner 35.12

imobachgs · 2024-12-05T12:24:16Z

Please, take into account that translations are merged once a week.

bittin · 2024-12-05T12:25:28Z

Please, take into account that translations are merged once a week.

ah alright did not know that, thank you for the information

bittin · 2024-12-06T07:56:30Z

@mvidner 35.12

this is now fixed in 36.3 thanks @imobachgs @dgdavid and @mvidner :)

## Problem During a review meeting on December 9th, it was argue that the [new screen for enforcing at least one root authentication method](#1787) could be intimidating for some users, mainly due to the inclusion of the [SSH public key](https://www.ssh.com/academy/ssh/public-key-authentication) input field. ## Solution In the same meeting, it was suggested to hide the SSH public key input and display it only upon user request, taking the opportunity for also providing a brief explanation of what an SSH public key is. While this was a good idea, after testing a couple of approaches it became apparent that there were several corner cases, all of them leading to add too much user interaction to an already intrusive screen that should be kept as minimalist as possible. Thus, the input has been completely removed instead. Now, the page ask for the root password and provides information to let users aware about the possibility to change the root authentication method at any time before installation from the _Users_ section. --- Related to https://trello.com/c/GQytAUEU (internal link)

Update to release version 11. * #1495 * #1564 * #1617 * #1618 * #1625 * #1626 * #1627 * #1628 * #1630 * #1631 * #1632 * #1633 * #1634 * #1635 * #1636 * #1639 * #1640 * #1641 * #1642 * #1643 * #1644 * #1645 * #1646 * #1647 * #1648 * #1649 * #1650 * #1651 * #1652 * #1654 * #1655 * #1656 * #1657 * #1660 * #1663 * #1666 * #1667 * #1668 * #1670 * #1671 * #1673 * #1674 * #1675 * #1676 * #1677 * #1681 * #1682 * #1683 * #1684 * #1687 * #1688 * #1689 * #1690 * #1691 * #1692 * #1693 * #1694 * #1695 * #1696 * #1698 * #1699 * #1702 * #1703 * #1704 * #1705 * #1707 * #1708 * #1709 * #1710 * #1711 * #1712 * #1713 * #1714 * #1715 * #1716 * #1717 * #1718 * #1720 * #1721 * #1722 * #1723 * #1727 * #1728 * #1729 * #1731 * #1732 * #1733 * #1734 * #1735 * #1736 * #1737 * #1740 * #1741 * #1743 * #1744 * #1745 * #1746 * #1751 * #1753 * #1754 * #1755 * #1757 * #1762 * #1763 * #1764 * #1765 * #1766 * #1767 * #1769 * #1771 * #1772 * #1773 * #1774 * #1777 * #1778 * #1785 * #1786 * #1787 * #1788 * #1789 * #1790 * #1791 * #1792 * #1793 * #1794 * #1795 * #1796 * #1797 * #1798 * #1799 * #1800 * #1802 * #1803 * #1804 * #1805 * #1807 * #1808 * #1809 * #1810 * #1811 * #1812 * #1814 * #1815 * #1821 * #1822 * #1823 * #1824 * #1825 * #1826 * #1827 * #1828 * #1830 * #1831 * #1832 * #1833 * #1834 * #1835 * #1836 * #1837 * #1838 * #1839 * #1840 * #1841 * #1842 * #1843 * #1844 * #1845 * #1847 * #1848 * #1849 * #1850 * #1851 * #1854 * #1855 * #1856 * #1857 * #1860 * #1861 * #1863 * #1864 * #1865 * #1866 * #1867 * #1871 * #1872 * #1873 * #1875 * #1876 * #1877 * #1878 * #1880 * #1881 * #1882 * #1883 * #1884 * #1885 * #1886 * #1888 * #1889 * #1890

With the authentication model now defined, stop enforcing the root password as a mandatory step that was hijacking the user workflow. This reverts the changes introduced in #1787 and #1821. Related to https://trello.com/c/7ewFvR0X (internal link)

jreidinger and others added 13 commits November 27, 2024 14:39

add initial code for routing to users if there is issue with user

f88c76c

remove trailing spaces

22b4e36

fix code to properly redirect to users

9f3fd52

runs prettier

7067817

make eslint happy

aca49a5

feat(web): add specific page for editing root user

764c9d1

Which will be initially shown after selecting a product if user issues are found.

feat(web): support specifiying title page from route

a068afb

Useful for routes "outside" of "installation settings context".

fix(web) small core/Page internal improvements

46bfe82

feat(web): add initial content for RootAuthMethodsPage

040d4de

Still missing the logic for validating and sending data provided by user.

Merge branch 'master' into users_mandatory_step

eda7a52

feat(web): don't render Install button at root auths path

3f4fa4b

feat(web): make RootAuthMethodsPage work

94113d0

And also * add a unit testing for ensuring it behaves as expected. * fix some typos found in previously touched files.

fix(web): replace passwordEncrypted by encryptedPassword

00da7b3

To keep consistency and not having both.

dgdavid changed the title ~~add initial code for routing to users if there is issue with user~~ feat(web): request for root authentication method after selecting a product Nov 28, 2024

dgdavid changed the title ~~feat(web): request for root authentication method after selecting a product~~ feat(web): request a root authentication method after selecting a product Nov 28, 2024

doc(web): update changes file

b0ac195

dgdavid reviewed Nov 28, 2024

View reviewed changes

web/src/queries/users.ts Show resolved Hide resolved

fix(rust): use encrypted_password instead of password_encrypted

bf10992

For consistency with the rest of the code. Related to 00da7b3

dgdavid marked this pull request as ready for review November 28, 2024 15:35

jreidinger commented Nov 28, 2024

View reviewed changes

web/src/App.tsx Outdated Show resolved Hide resolved

jreidinger commented Nov 28, 2024

View reviewed changes

web/src/components/core/Page.tsx Show resolved Hide resolved

jreidinger commented Nov 28, 2024

View reviewed changes

dgdavid added 2 commits November 29, 2024 08:14

doc(web): improve core/Page documentation

0e61251

By fixing typos and rewording documentation props. Also drops a no longer needed TODO at src/App.tsx

Merge branch 'master' into users_mandatory_step

62a8a1c

dgdavid requested a review from imobachgs November 29, 2024 08:17

This comment was marked as outdated.

Sign in to view

dgdavid and others added 2 commits November 29, 2024 09:02

fix(web): rewording from code review

93ea0af

Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>

fix(web): use camelCase instead of snake_case

a2068d5

mvidner reviewed Nov 29, 2024

View reviewed changes

web/src/components/users/RootAuthMethodsPage.tsx Outdated Show resolved Hide resolved

fix(web): drop test related to useLocation

5513b2a

Because the mock was affecting to the entire test suite. They will be reintroduced once the react-router-dom mock is better done.

dgdavid reviewed Nov 29, 2024

View reviewed changes

web/src/components/users/RootAuthMethodsPage.test.tsx Outdated Show resolved Hide resolved

doc(web): fix typo

be2f6db

This comment was marked as resolved.

Sign in to view

doc(web): update changelog files

6c09486

imobachgs approved these changes Dec 1, 2024

View reviewed changes

dgdavid requested a review from imobachgs December 1, 2024 22:39

Merge branch 'master' into users_mandatory_step

95e50c6

This comment was marked as resolved.

Sign in to view

imobachgs approved these changes Dec 2, 2024

View reviewed changes

dgdavid merged commit 1376214 into master Dec 2, 2024
12 checks passed

dgdavid deleted the users_mandatory_step branch December 2, 2024 07:20

mvidner reviewed Dec 2, 2024

View reviewed changes

dgdavid mentioned this pull request Dec 9, 2024

refactor(web): simplify page to enforce root password #1821

Merged

imobachgs mentioned this pull request Jan 10, 2025

Update to release 11 #1887

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(web): request a root authentication method after selecting a product #1787

feat(web): request a root authentication method after selecting a product #1787

jreidinger commented Nov 27, 2024 •

edited by dgdavid

Loading

jreidinger Nov 28, 2024

dgdavid Nov 28, 2024

mvidner Nov 29, 2024

dgdavid Nov 29, 2024

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

mvidner Dec 2, 2024

bittin Dec 2, 2024

bittin commented Dec 3, 2024

bittin commented Dec 5, 2024

dgdavid commented Dec 5, 2024

bittin commented Dec 5, 2024

mvidner commented Dec 5, 2024

mvidner commented Dec 5, 2024

bittin commented Dec 5, 2024

imobachgs commented Dec 5, 2024

bittin commented Dec 5, 2024

bittin commented Dec 6, 2024

feat(web): request a root authentication method after selecting a product #1787

feat(web): request a root authentication method after selecting a product #1787

Conversation

jreidinger commented Nov 27, 2024 • edited by dgdavid Loading

Problem

Solution

Notes for reviewers

Testing

Screenshots

jreidinger Nov 28, 2024

Choose a reason for hiding this comment

dgdavid Nov 28, 2024

Choose a reason for hiding this comment

mvidner Nov 29, 2024

Choose a reason for hiding this comment

dgdavid Nov 29, 2024

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

mvidner Dec 2, 2024

Choose a reason for hiding this comment

bittin Dec 2, 2024

Choose a reason for hiding this comment

bittin commented Dec 3, 2024

bittin commented Dec 5, 2024

dgdavid commented Dec 5, 2024

bittin commented Dec 5, 2024

mvidner commented Dec 5, 2024

mvidner commented Dec 5, 2024

bittin commented Dec 5, 2024

imobachgs commented Dec 5, 2024

bittin commented Dec 5, 2024

bittin commented Dec 6, 2024

jreidinger commented Nov 27, 2024 •

edited by dgdavid

Loading