Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Check of write permission of atKeys file path when submitting enroll request #690

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/at_onboarding_cli/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
## 1.7.1
- When submitting an enrollment request, check for write permissions of AtKeys file path.
Copy link
Contributor

Choose a reason for hiding this comment

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

add 'fix: ' at the start of this comment please

Copy link
Member Author

Choose a reason for hiding this comment

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

Added "fix:" in the changelog.md

## 1.7.0
- feat: add `auto` command to the activate CLI
## 1.6.4
Expand Down
58 changes: 56 additions & 2 deletions packages/at_onboarding_cli/lib/src/cli/auth_cli.dart
Original file line number Diff line number Diff line change
Expand Up @@ -350,8 +350,16 @@ Future<void> enroll(ArgResults argResults, {AtOnboardingService? svc}) async {
}

File f = File(argResults[AuthCliArgs.argNameAtKeys]);
if (f.existsSync()) {
stderr.writeln('Error: atKeys file ${f.path} already exists');
gkc marked this conversation as resolved.
Show resolved Hide resolved

// "_isWritable" attempts to create the directories for the given [file] if they do not exist,
// and then tries to open the file in write mode to verify write permissions.
//
// If the file can be opened for writing, it is immediately closed and deleted.
// In [AtOnboardingServiceImpl._generateAtKeysFile] method, there is a check which returns error
// if the file already exists. Therefore, delete the file here after checking for write permissions.
//
// Incase of any exceptions, the error is logged and returned.
if (isWritable(f) == false) {
return;
}

Expand Down Expand Up @@ -408,6 +416,52 @@ Future<void> enroll(ArgResults argResults, {AtOnboardingService? svc}) async {
}
}

/// Checks if the specified [file] is writable.
///
/// This function attempts to create the directories for the given [file] if they do not exist,
/// and then tries to open the file in write mode to verify write permissions.
/// If the file can be opened for writing, it is immediately closed and deleted.
///
/// Returns:
/// - `true` if the file is writable, or
/// - `false` if the file is not writable due to existing files, lack of permissions,
/// or any other exceptions encountered during the process.
///
/// [file]: The [File] instance to check for write access.
///
/// Exceptions:
/// - If the file already exists, a [PathExistsException] is caught, and an error message is printed to stderr.
/// - If the file does not have write permissions, a [PathAccessException] is caught, and an error message is printed to stderr.
/// - Any other exceptions are caught and logged, indicating a failure to determine write access.
@visibleForTesting
bool isWritable(File file) {
try {
// If the directories do not exist, create them.
// "recursive" is set to true to ensure that any missing parent directories are created.
// "exclusive" is set to true to check if the file already exists; if it does,
// an error will be logged and returned.
file.createSync(recursive: true, exclusive: true);
// Try opening the file in write mode, which requires write permissions
RandomAccessFile raf = file.openSync(mode: FileMode.write);
raf.closeSync();
// In [AtOnboardingServiceImpl._generateAtKeysFile] method, there is a check which returns error
// if the file already exists. Therefore, delete the file here after checking for write permissions.
file.deleteSync();
return true;
} on PathExistsException {
stderr.writeln('Error: atKeys file ${file.path} already exists');
return false;
} on PathAccessException {
stderr.writeln(
'Error : atKeys file ${file.path} does not have write permissions');
return false;
} catch (e) {
// If any exception occurs, we assume the file is not writable
stderr.writeln('Error in writing to atKeys file: ${e.toString()}');
return false;
}
}

@visibleForTesting
Future<void> setSpp(ArgResults argResults, AtClient atClient) async {
String spp = argResults[AuthCliArgs.argNameSpp];
Expand Down
2 changes: 1 addition & 1 deletion packages/at_onboarding_cli/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: at_onboarding_cli
description: Dart tools for initial client onboarding, subsequent client enrollment, and enrollment management.
version: 1.7.0
version: 1.7.1
repository: https://github.com/atsign-foundation/at_libraries
homepage: https://atsign.com
documentation: https://docs.atsign.com/
Expand Down
43 changes: 43 additions & 0 deletions packages/at_onboarding_cli/test/auth_cli_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import 'dart:io';

import 'package:at_onboarding_cli/src/cli/auth_cli.dart';
import 'package:test/test.dart';

void main() {
final baseDirPath = 'test/keys';
group('A group of tests to verify write permission of apkam file path', () {
final dirPath = '$baseDirPath/@alice-apkam-keys.atKeys';

test(
'A test to verify isWritable returns false if directory has read-only permissions',
() async {
final directory = Directory(dirPath);
// Create the directory first to ensure it exists before calling isWritable.
await directory.create(recursive: true);
// Set permission to read only.
await Process.run('chmod', ['444', baseDirPath]);
expect(isWritable(File(dirPath)), false);
});

test(
'A test to verify isWritable returns false if file already exists in the directory',
() async {
final directory = Directory(dirPath);
// Create the directory first to ensure it exists before calling isWritable.
await directory.create(recursive: true);
expect(isWritable(File(dirPath)), false);
});

test(
'A test verify isWritable returns true if directory does not have a file already',
() {
expect(isWritable(File(dirPath)), true);
});
});

tearDown(() async {
// Set full permissions to delete the directory.
await Process.run('chmod', ['777', baseDirPath]);
Directory(baseDirPath).deleteSync(recursive: true);
});
}