Changed deSEC controll flow to allow for using existing subdomains with Authkey #2080

wildd0g · 2021-08-15T10:54:36Z

Put individual steps of script into functions, and the functions in a controll flow at the bottom.
Added a path to the control flow for people who already have a subdomain claimed.
Changed some of the text to accomodate the new controll flow.
Added general abbort-exit function, which displays instructions how to run the script at a later time.

#2077

enoch85 · 2021-08-15T18:49:15Z

Haven't tested yet, but please don't use tab, use 4 spaces for indentation.

Thanks!

wildd0g · 2021-08-18T10:01:09Z

Fixed

enoch85 · 2021-08-18T17:36:26Z

Thanks, I will have a look at this tomorrow or Friday.

enoch85 · 2021-08-19T10:07:30Z

Looking at it today and just tested with my own domain, using the token from DEDYN_TOKEN in the deSEC folder. Got this:

Maybe we need to generate a new token after all?

enoch85 · 2021-08-19T10:09:26Z

Aaah, you need to generate a new token from within your admin panel. We should add that, will fix!

wildd0g · 2021-08-19T10:10:42Z

Tob e honest, I simply asumed the AUTH_token you get from first setup is the same as the one you get sent in the e-mail. If it isn’t, I would need to look more into the process deDYN uses.

wildd0g · 2021-08-19T10:11:12Z

Ah, nice, I can leave that to you then?

enoch85 · 2021-08-19T10:12:10Z

Well, if we could automate it, it would be great, but I'm not sure that's possible. You can help me come up with an idea to fix it if you have time?

wildd0g · 2021-08-19T10:18:56Z

I’m reading into the deSEC documentation at the moment. Maybe this is usefull? https://desec.readthedocs.io/en/latest/auth/tokens.html

< 8000 !-- '"` -->

enoch85 · 2021-08-19T10:22:55Z

What we would like is to generate a new TOKEN by email, so that the user can add that in the same way as a new domain, but for an existing account.

wildd0g · 2021-08-19T10:28:13Z

I believe proper procedure would be testing if the provided token is a valid key management token in some way, then use the creating a Token procedure as described there to create a new one. Or if it is not one, testing if it is valid for managing the IP address assigned to a/the domain. But that is in the case that the person owns to domain and has a valid token ready. If they own the domain but don’t have a valid token ready, I that would lend itself to the situation where they would be sent a new valid one per e-mail. Let me play with the API a bit and see what I come up with.

enoch85 · 2021-08-19T11:51:26Z

I'm quite happy with the flow right now. If we can automate the flow, sure, but the current way also works as we instruct the user how to do it.

@szaimen If you have time to test it would be appreciated.

wildd0g · 2021-08-19T11:56:56Z

Is the existing account but not existing subdomain flow correct? It's missing registering the new domain claiming at the moment as far as i can tell...

enoch85 · 2021-08-19T12:33:51Z

Is the existing account but not existing subdomain flow correct? It's missing registering the new domain claiming at the moment as far as i can tell...

I'm not sure I follow. Can you ellaborate, or actually show what you mean?

wildd0g · 2021-08-19T12:49:47Z

Going throught he controll flow, lats assume the domain is free and they have an account:

while :
do
    prompt_dedyn_subdomain
    # Check for SOA record
    #Let's assume the domain is free:
    if host -t SOA "$DEDYNDOMAIN" >/dev/null 2>&1
    then
        <skipped>
    else

This following check seems to be wrong, if the user entered "no" They don't have an existing account, then the negation turns it into true and it follows the Register with existing account flow?
Any way, let's assume it goes into the Register with existing account flow:

        # Domain is free and available to register
        if ! existing_account
        then
            # Register with existing account
            break
        else
            <skipped>
        fi
    fi
done

prompt_security_token
prompt_dyndns
prompt_tls

So the steps it actually executes is:

prompt_dedyn_subdomain -> ask which subdomain it is for
Check that it is taken (it is not)
existing_account -> check if the user claims to have an existing account (they answer yes and are instructed to make a new auth token)
prompt_security_token -> they enter this token
prompt_dyndns -> dynDNS is activated to use the provided token
prompt_tls -> TLS is activated to use the provided token

Nowhere in that list is the domain, confirmed to be free, actually claimed by/assigned to the user on deSEC's service, right?
I think that is a crucial missing step.
The other situation (domain taken) should be able to skip that, since it is taken by the user in question.

enoch85 · 2021-08-19T13:14:41Z

The argument is changed in the latest version, please check again.

Still, we have a loop going on, working on it now.

wildd0g · 2021-08-19T13:32:06Z

I'm going to really rework this script, it currently functions off of claiming the domain at account creation on deSEC, and using the included account recovery authtoken for the rest.

That is also the source of the weird requirement that any new domain required an as of yet unused email.

I envision the best way to do it would be:

Ask if user has an account (yes/no):
    If no:
        Prompt for credentials (email/password) to create account with (and ask to activate it/complete Captcha in first login).
        Auto login using entered credentials (generates login authtoken)
    if yes:
        Prompt for credentials to log in with
        Auto login (generates login authtoken)
Prompt subdomain from user.
Check if subdomain free
    if yes:
         register new subdomain (Check for maximum amount of domains registered is exceeded)
    if no:
        Check if it exists in subdomains associated with account
        if yes:
            break
        if no:
            Prompt for different subdomain or abort

Ask user to set up DynDNS (yes/no)
    if yes:
        Generate new token for DynDNS using API
        Current DynDNS setup using this new token
        
Ask user to set up TLS (yes/no)
    if yes:
        Generate new token for TLS using API
        Current TLS  setup using this new token

Invalidate/delete current login token

enoch85 · 2021-08-19T14:39:18Z

@wildd0g Found any solution to the email issue, or do you think that the current one works?

wildd0g · 2021-08-19T14:41:05Z

As i mentioned, currently reworking the script/flow.

Getting the login functionality working is not as easy, extracting the token from the responce stored to file is providing a bit of a challenge.

enoch85 · 2021-08-19T14:41:59Z

As i mentioned, currently reworking the script/flow.

But it works as it is right now, at least in my testing. Anything missing you think?

enoch85 · 2021-08-19T15:49:41Z

You can also assume that 99% of the users doesn't have an account with deSEC, so that shouldn't be a priority/first question.

wildd0g · 2021-08-19T15:53:33Z

Mostly that horrible behavior of not being able to add new domains to an existing account currently, due to the fact domain claiming is done by using the added 'domain' option in the account creation API.

I'm 90% done with a reworked version that separates out the domain claiming from account creation. Since the control flow is also separated from the rest, changing the order later to favor first checking domain availability and then account existence should be trivial.

enoch85 · 2021-08-19T16:50:05Z

OK, gotcha. 👍 If you're interested in investing some time, maybe you could have a look at this as well?

#1994

wildd0g · 2021-08-19T18:08:41Z

So while playing around, I believe i got the
run_script NETWORK ddclient-configuration
part to fail, since this git repo is not in /var/scripts in my development environment.
But there are some things that are important to clean up, like the auth keys stored as variables, and I believe the cleanup portion did not trigger on that crash.
Van/do we have to do something about that?

wildd0g · 2021-08-19T18:19:20Z

By the way, do you have an occupied domain that I can test the "domain taken that you do not own" behavior against?

enoch85 · 2021-08-19T18:43:13Z

Van/do we have to do something about that?

Change the repo to to yours for the testing part, all the scripts exists in your fork as well. In lib.sh there are several mentions, and then you need to change the source URL as well.

do you have an occupied domain that I can test the "domain taken that you do not own" behavior against?

I just test random domains and delete them afterwards, either manually, or with the delete account script included in the deSEC menu.

wildd0g · 2021-08-19T23:38:10Z

Well, I believe it finally works as intended.

Using while: do and continue as a substitute for a goto feels so weird/dirty, but if can't do it as you should, then you should do it as you can.
Same with repeated

if prompt_try_different_domain
    then
        continue
    else
        aborted_exit_message
    fi

blocks.

I put them in a function at first, but apparently doing a break or continue in a function does not propagate to the outer loop.

Have a look, the behavior should be as I described above.

Also something to note, the login generates a token that is valid for 1 day and gets removed after 7 (if i understand deSEC right) which can manage other tokens.
The tokens generated for the DynDNS and TLS are valid indefinitely, get their own custom names, but don't have the rights to manage other tokens.

Signed-off-by: szaimen <szaimen@e.mail.de> Signed-off-by: wildd0g <mpeppelman2008@hotmail.com>

Signed-off-by: wildd0g <mpeppelman2008@hotmail.com>

1. Since the script now works with existing accounts, make a new function to ask for that, and use it in the final composition. 2. Domain available? Ask for existing account to register it with 3. Domain taken? Ask for existing account to register it with Signed-off-by: wildd0g <mpeppelman2008@hotmail.com>

Signed-off-by: wildd0g <mpeppelman2008@hotmail.com>

wildd0g · 2021-08-20T00:24:33Z

So that was an entire mess.

I signed off all your previous commits as well, since DCO was complaining about them.
See if the new version is acceptable, otherwise we can revert to
d4b811f
if needed.

enoch85 · 2021-08-20T07:57:33Z

This feel uncomfortable to merge.

Please revert to the proposed commit. That works at least. I'll merge that, and then we start over with this.

enoch85 · 2021-08-20T08:35:52Z

We can continue to work in this, I'll merge the changes from the original idea in another PR: #2088

You can start clean backing up your changes, then create a new PR to get a clean start.

enoch85 · 2021-08-20T08:43:55Z

Did a test now with no account, and no domain. Noticed this:

desec.sh: line 190: -Po: command not found.

Also, TLS doesn't work as the hook seems to be gone. You changed the name of the most important token to RETURNTOKEN, please revert that change, else we have to change in several scripts and I see no reason to change the name of that variable.

enoch85 · 2021-08-20T08:49:12Z

But I like the idea, instead of registering the domain, we register an account, and connects the domain to that account. 👍

Some code styling, and minor fixes, but generally great!

enoch85 · 2021-08-20T08:57:31Z

We should also implement this:

# Check if deSEC is already installed
print_text_in_color "$ICyan" "Checking if deSEC is already installed..."
if ! is_desec_installed
then
    # Ask for installing
    install_popup "$SCRIPT_NAME"
else
    # Ask for removal or reinstallation
    reinstall_remove_menu "$SCRIPT_NAME"
    # Removal
    run_script ADDONS remove_desec
    removal_popup "$SCRIPT_NAME"
fi

It requires that we remove some of the assumptions in the removal script, or simply copy the code from the removal script to the main install script.

wildd0g · 2021-08-20T10:33:38Z

So i did that revert to d4b811f you asked for, but that also undid a merge you performed?

wildd0g · 2021-08-20T10:47:13Z

We should also implement this:
# Check if deSEC is already installed
print_text_in_color "$ICyan" "Checking if deSEC is already installed..."
if ! is_desec_installed
then
    # Ask for installing
    install_popup "$SCRIPT_NAME"
else
    # Ask for removal or reinstallation
    reinstall_remove_menu "$SCRIPT_NAME"
    # Removal
    run_script ADDONS remove_desec
    removal_popup "$SCRIPT_NAME"
fi
It requires that we remove some of the assumptions in the removal script, or simply copy the code from the removal script to the main install script.

I'm... confused, what functionality do you want to add with that?

~~Also, I'm taking a look at the subdomain and removal scripts, they call 'is_desec_installed' but i can't seem to find where that is defined?~~
Never mind, found it in the lib script.
Would it be preferable to put some more general functionality, like adding/removing domains, in lib as opposed to in each specific script?

enoch85 · 2021-08-20T11:02:54Z

So i did that revert to d4b811f you asked for, but that also undid a merge you performed?

Remove your fork, delete the repo. Git pull fresh, and start over. But first, (let me take a selife) copy your current code in desec.sh so that you can add that back on the fresh clone.

Would it be preferable to put some more general functionality, like adding/removing domains, in lib as opposed to in each specific script?

Yes, I would like it to work as with "Activate TLS" where we make a function out of the TLS stuff for deSEC so that we can use that repo wide if needed, in the same way as with the regular TLS. I made a script for adding subdomains, but I'm not sure that works with Collabora|Onlyoffice|Talk and other apps that needs TLS on a separate subdoman. I wrote a list here: #1994

The removal function is great, but we need to implement it here as well as the removal script is assumed to be run standalone.

enoch85 · 2021-08-20T11:10:05Z

Also, yes, generally we put all functions into lib.sh, but in this case I see no need for it as they're only used in desec.sh. If there are functions that are used in more places, they should go into lib.sh.

Thinking about it, one is actually aborted_error_message, that would go into lib, since we use the same text elsewhere as well.

wildd0g · 2021-08-20T11:13:20Z

Just to confirm, I'm to remove my fork entirely and start from scratch? In stead of closing this pull request, update my fork to the newest master, and continue onward with my branch in another pull request?

enoch85 · 2021-08-20T11:36:20Z

Just to confirm, I'm to remove my fork entirely and start from scratch? In stead of closing this pull request, update my fork to the newest master, and continue onward with my branch in another pull request?

Do as you wish, but we need a clean slate here. As long as it's the latest master, I'm fine with it. 👍

wildd0g · 2021-08-20T12:21:33Z

👍Closing this PR

enoch85 · 2022-08-18T21:27:32Z

@wildd0g Hey, just wondering if you ha any plans on improving deSEC further? You talked about a refactor IIRC.

wildd0g · 2022-08-19T16:26:49Z

I'm currently moving house, afterwards I can take another look. Refactor was focused on cotroll flow for assigning and auto updating a domain name, since the flow as it was assumed you need to create a new one and had no account yet, so situations where you already had an account and/or already had a domain you wanted to re-use were not supported.

…

________________________________ Van: Daniel Hansson ***@***.***> Verzonden: donderdag 18 augustus 2022 23:27 Aan: nextcloud/vm ***@***.***> CC: wildd0g ***@***.***>; Mention ***@***.***> Onderwerp: Re: [nextcloud/vm] Changed deSEC controll flow to allow for using existing subdomains with Authkey (#2080) @wildd0g<https://github.com/wildd0g> Hey, just wondering if you ha any plans on improving deSEC further? You talked about a refactor IIRC. — Reply to this email directly, view it on GitHub<#2080 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACJIBM43YNSDGYSPF6LPCW3VZ2TE5ANCNFSM5CGB2C4Q>. You are receiving this because you were mentioned.Message ID: ***@***.***>

enoch85 · 2022-08-20T15:58:05Z

OK, no stress. Just switched house myself this month, I know the work load!

wildd0g force-pushed the wd-dev branch from 1535d0c to 9080b24 Compare August 15, 2021 11:55

wildd0g changed the title ~~Resolves #2077~~ Changed deSEC controll flow to allow for using existing subdomains with Authkey Aug 15, 2021

wildd0g force-pushed the wd-dev branch from 25ed47f to 44ac36c Compare August 20, 2021 00:17

szaimen and others added 9 commits August 20, 2021 00:22

improve logging of the bitwarden to vaultwarden update (nextcloud#2083)

0cab418

Signed-off-by: szaimen <szaimen@e.mail.de> Signed-off-by: wildd0g <mpeppelman2008@hotmail.com>

move spamhaus-drop to our repo (nextcloud#2085)

8e0314b

Signed-off-by: wildd0g <mpeppelman2008@hotmail.com>

details

0d28963

Signed-off-by: wildd0g <mpeppelman2008@hotmail.com>

fix if argument for existing account

31ba5a8

Signed-off-by: wildd0g <mpeppelman2008@hotmail.com>

show message if existing

bfe9dc3

Signed-off-by: wildd0g <mpeppelman2008@hotmail.com>

fix bugs

fbf4b15

Signed-off-by: wildd0g <mpeppelman2008@hotmail.com>

fine tuning

0adc1b0

Signed-off-by: wildd0g <mpeppelman2008@hotmail.com>

typo, thanks shellcheck!

d4b811f

Signed-off-by: wildd0g <mpeppelman2008@hotmail.com>

wildd0g force-pushed the wd-dev branch from 674a761 to 0d9a697 Compare August 20, 2021 00:22

enoch85 mentioned this pull request Aug 20, 2021

allow existing user to use deSEC #2088

Merged

wildd0g force-pushed the wd-dev branch from 40f80e6 to d4b811f Compare August 20, 2021 10:29

Merge branch 'master' into wd-dev

48510e5

wildd0g closed this Aug 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changed deSEC controll flow to allow for using existing subdomains with Authkey #2080

Changed deSEC controll flow to allow for using existing subdomains with Authkey #2080

Changed deSEC controll flow to allow for using existing subdomains with Authkey #2080

Changed deSEC controll flow to allow for using existing subdomains with Authkey #2080

Conversation